refactor stateless checks to be done on types with withdrawals #1430
Comments
github-merge-queue bot
pushed a commit
that referenced
this issue
Aug 29, 2024
) ## Summary Adds state for bridge events and consumes the events as they occur. Updates proto for bridge unlock to no longer use memo for rollup information and instead include on main action. Renames `rollup_transaction_hash` to `rollup_withdrawal_event_id`. ## Background There was only implicit stop against reusing a withdrawal event, this adds consumption of withdrawal events. While this is not strictly security enhancing, consuming the event adds in protocol protection against accidental double spend by the bridge operator. ## Changes - Proto updates for `BridgeUnlockAction` + `ICS20WithdrawalMemo` - Add stateless checks on bridging unlock and withdrawals of correct information filled out - Added state to enforce rollup event cannot be consumed twice. - Enforce that `Ics20WithdrawalAction` must have `bridge_address` set if making a bridge withdrawal ## Testing smoke test + minor test updates ## Breaking Changelist - New stateful information about rollup withdrawal events - Adds new fields to `BridgeUnlockAction` ## Related Issues closes #1430 --------- Co-authored-by: Fraser Hutchison <[email protected]> Co-authored-by: Richard Janis Goldschmidt <[email protected]>
jbowen93
pushed a commit
that referenced
this issue
Sep 3, 2024
) ## Summary Adds state for bridge events and consumes the events as they occur. Updates proto for bridge unlock to no longer use memo for rollup information and instead include on main action. Renames `rollup_transaction_hash` to `rollup_withdrawal_event_id`. ## Background There was only implicit stop against reusing a withdrawal event, this adds consumption of withdrawal events. While this is not strictly security enhancing, consuming the event adds in protocol protection against accidental double spend by the bridge operator. ## Changes - Proto updates for `BridgeUnlockAction` + `ICS20WithdrawalMemo` - Add stateless checks on bridging unlock and withdrawals of correct information filled out - Added state to enforce rollup event cannot be consumed twice. - Enforce that `Ics20WithdrawalAction` must have `bridge_address` set if making a bridge withdrawal ## Testing smoke test + minor test updates ## Breaking Changelist - New stateful information about rollup withdrawal events - Adds new fields to `BridgeUnlockAction` ## Related Issues closes #1430 --------- Co-authored-by: Fraser Hutchison <[email protected]> Co-authored-by: Richard Janis Goldschmidt <[email protected]>
jbowen93
pushed a commit
that referenced
this issue
Sep 6, 2024
) ## Summary Adds state for bridge events and consumes the events as they occur. Updates proto for bridge unlock to no longer use memo for rollup information and instead include on main action. Renames `rollup_transaction_hash` to `rollup_withdrawal_event_id`. ## Background There was only implicit stop against reusing a withdrawal event, this adds consumption of withdrawal events. While this is not strictly security enhancing, consuming the event adds in protocol protection against accidental double spend by the bridge operator. ## Changes - Proto updates for `BridgeUnlockAction` + `ICS20WithdrawalMemo` - Add stateless checks on bridging unlock and withdrawals of correct information filled out - Added state to enforce rollup event cannot be consumed twice. - Enforce that `Ics20WithdrawalAction` must have `bridge_address` set if making a bridge withdrawal ## Testing smoke test + minor test updates ## Breaking Changelist - New stateful information about rollup withdrawal events - Adds new fields to `BridgeUnlockAction` ## Related Issues closes #1430 --------- Co-authored-by: Fraser Hutchison <[email protected]> Co-authored-by: Richard Janis Goldschmidt <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR #1391 introduces some stateless checks in
BridgeUnlockActionandIcs20WithdrawalActionwhich are very similar. This is done because of parsing done in stateless check of memo forIcs20WithdrawalAction.As a follow up we should update the canonical domain rust types to doing this validation during parsing, ideally there is some shared logic so it can be kept insync between the two actions.
┆Issue Number: ENG-766
The text was updated successfully, but these errors were encountered: