fix(core, bridge, sequencer)!: dismabiguate return addresses #1266
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
SuperFluffy
changed the title
… ics20 withdrawals
steezeburger
added a commit
that referenced
this pull request
Jul 15, 2024
* main: feat(cli): add cmd to collect withdrawal events and submit as actions (#1261) fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266) fix(withdrawer): support withdrawer address that differs from bridge address (#1262) (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260) fix(charts): add resources for sequencer/cometbft (#1254) chore(sequencer)!: add metrics (#1248) fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243) feat(core, proto)!: make bridge unlock memo string (#1244) fix(conductor): don't panic during panic (#1252) feat(core)!: lowerCamelCase for protobuf json mapping (#1250) refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190) fix: rollup archive node configurations (#1249) refactor(core, bridge-withdrawer)!: move bridge-unlock memo to core (#1245) fix(sequencer)!: store native asset ibc->trace mapping in init_chain (#1242)
steezeburger
added a commit
that referenced
this pull request
Jul 19, 2024
* main: (24 commits) chore: update `bytes` and `ics23` crates (#1279) fix(sequencer): improve and fix instrumentation (#1255) feature(charts): hermes chart fixes, bech32 updates, ibc bridge test (#1130) chore(cli): remove unused rollup cli code (#1275) chore(test): use a temporary file to not pollute the workspace (#1269) chore(sequencer): add mempool benchmarks (#1238) fix(bridge-withdrawer)!: fix nonce handling (#1215) feat(cli, bridge-withdrawer)!: share code between cli and service (#1270) feat(cli): add cmd to collect withdrawal events and submit as actions (#1261) fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266) fix(withdrawer): support withdrawer address that differs from bridge address (#1262) (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260) fix(charts): add resources for sequencer/cometbft (#1254) chore(sequencer)!: add metrics (#1248) fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243) feat(core, proto)!: make bridge unlock memo string (#1244) fix(conductor): don't panic during panic (#1252) feat(core)!: lowerCamelCase for protobuf json mapping (#1250) refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190) fix: rollup archive node configurations (#1249) ...
bharath-123
pushed a commit
that referenced
this pull request
Jul 25, 2024
## Summary Ensures that bridge-withdrawer does not use a rollup address as a sequencer refund address. ## Background Bridge-withdrawer was using an address on the ethereum rollup as the sequencer refund address in the actions it was submitting to sequencer. That was wrong. ## Changes - add a field `rollup_refund_address` to `Ibc20WithdrawalFromRollupMemo` to make its purpose clear - In bridge-withdrawer, set `refund_address` to the bridge account always (`refund_address` and `bridge_address` are now the same). - Sequencer now assumes that for refunds of ics20 withdrawals initiated by a rollup `FungibleToketPacketData.sender` is now the bridge address to which funds should be refunded. This is consistent with the change above. - Remove field `Ibc20WithdrawalFromRollupMemo.brigde_address` as it is no longer necessary. ## Testing Tests were updated and pass. This patch relies on the tests exhaustively testing the ics20 logic. ## Breaking Changelist - This change is network-breaking because `Ibc20WithdrawalFromRollupMemo` is used in Sequencer's protocol.
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.
Summary
Ensures that bridge-withdrawer does not use a rollup address as a sequencer refund address.
Background
Bridge-withdrawer was using an address on the ethereum rollup as the sequencer refund address in the actions it was submitting to sequencer. That was wrong.
Changes
rollup_refund_addresstoIbc20WithdrawalFromRollupMemoto make its purpose clearrefund_addressto the bridge account always (refund_addressandbridge_addressare now the same).FungibleToketPacketData.senderis now the bridge address to which funds should be refunded. This is consistent with the change above.Ibc20WithdrawalFromRollupMemo.brigde_addressas it is no longer necessary.Testing
Tests were updated and pass. This patch relies on the tests exhaustively testing the ics20 logic.
Breaking Changelist
Ibc20WithdrawalFromRollupMemois used in Sequencer's protocol.