fix: only allow whitelisted relayer account to send IBC relayer messages #614
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
keruch
previously approved these changes
Dec 6, 2024
|
@omritoptix we wanted whitelisted to allow client creation |
omritoptix
requested changes
Dec 6, 2024
yea true but don't think it's worth extra effort. mostly thought it's gonna reduce work. |
omritoptix
previously approved these changes
Dec 8, 2024
danwt
reviewed
Dec 8, 2024
Comment on lines
46
to
67
| msgs, err = n.getAllFinalMsgs(ctx, msgs, 0) | ||
| if err != nil { | ||
| // This error is not critical; just log and fall into the default fee handling | ||
| ctx.Logger().With("module", "BypassIBCFeeDecorator", "err", err). | ||
| Error("Failed to check if the tx is from the whitelisted relayer") | ||
| return ctx, err | ||
| } | ||
| if whitelisted { | ||
| // The tx is from the whitelisted relayer, so it's eligible for the fee exemption | ||
| return next(ctx, tx, simulate) | ||
|
|
||
| totalMsgs := len(msgs) | ||
| ibcCount := countIBCMsgs(msgs) | ||
|
|
||
| if ibcCount == totalMsgs { | ||
| // all are IBC messages | ||
| ibcWhitelisted, err := n.isIBCWhitelistedRelayer(ctx, msgs) | ||
| if err != nil { | ||
| return ctx, err | ||
| } | ||
| if ibcWhitelisted { | ||
| return next(ctx, tx, simulate) | ||
| } | ||
| return ctx, fmt.Errorf("not all signers whitelisted") | ||
| } else if ibcCount > 0 { | ||
| // mixed: some IBC and some non-IBC | ||
| return ctx, fmt.Errorf("mixed IBC and non-IBC messages in the same transaction not allowed") | ||
| } |
There was a problem hiding this comment.
Are you sure this is gonna let IBC gov proposals work as normal?
There was a problem hiding this comment.
if a governance proposal includes IBC messages and the signer is not whitelisted, the code will reject the transaction
…telisted-accs-from-creating-ibc-client
omritoptix
approved these changes
Dec 8, 2024
omritoptix
deleted the
zale144/block-non-whitelisted-accs-from-creating-ibc-client
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes #615
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleasedsection inCHANGELOG.mdgodoccommentsSDK Checklist
maptime.Now()sendCoinand notSendCoinsFull security checklist here
For Reviewer:
After reviewer approval: