Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sequencer): implement refund to rollup logic upon ics20 transfer refund #1161

Merged
merged 12 commits into from
Jun 8, 2024

Conversation

noot
Copy link
Collaborator

@noot noot commented Jun 7, 2024

Summary

implement refund to rollup logic upon ics20 transfer refund, if the transfer originally can from an Ics20Withdrawal that corresponded to a rollup withdrawal

Background

we want to users to get their funds back in on the rollup automatically if they bridge out via IBC and it fails. currently, if bridging out from the rollup via IBC fails, their funds are returned on the sequencer, which would require the user to then manually do another BridgeLock if they want their funds back on the rollup.

Changes

  • implement logic in the ics20 transfer module which detects if a transfer is a refund originating from a rollup withdrawal, and if it is, transfers the funds to the specified bridge account and emits a Deposit event.
  • update the withdrawer to be aware of the bridge account on the sequencer which corresponds to the rollup asset it's bridging, and it needs to place this in the withdraw memo in case of failure.

Testing

unit tests

Related Issues

closes #1110

@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Jun 7, 2024
Comment on lines 38 to 42
ASTRIA_BRIDGE_WITHDRAWER_ROLLUP_ASSET_DENOMINATION="nria"

# The hex-encoded bridge address corresponding to the bridged rollup asset on the sequencer.
# Should match the bridge address in the geth rollup's bridge configuration for that asset.
ASTRIA_BRIDGE_WITHDRAWER_SEQUENCER_BRIDGE_ADDRESS=""
Copy link
Collaborator Author

@noot noot Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i want to add these as immutable vars to the withdrawal contract instead of config values, will do in a follow up.

@noot noot marked this pull request as ready for review June 7, 2024 18:26
@noot noot requested review from a team as code owners June 7, 2024 18:26
@noot noot requested review from Fraser999 and joroshiba June 7, 2024 18:26
@noot noot enabled auto-merge June 8, 2024 22:58
@noot noot added this pull request to the merge queue Jun 8, 2024
Merged via the queue into main with commit cd5dbd0 Jun 8, 2024
38 checks passed
@noot noot deleted the noot/ibc-bridge-timeout branch June 8, 2024 23:08
steezeburger added a commit that referenced this pull request Jun 10, 2024
* main:
  fix: ignore RUSTSEC-2021-0139 (#1171)
  chore(sequencer-relayer)!: remove functionality to restrict relaying blocks to only those proposed by a given validator (#1168)
  chore(metrics): update `metric_name` macro to handle a collection of names (#1163)
  fix(bridge-withdrawer): skip linting generated contract code (#1172)
  fix(core, sequencer): prefix removal source non-refund ics20 packet (#1162)
  chore(docs): add sequencer-relayer doc to specs (#1126)
  feat(bridge-withdrawer): sync logic (#1165)
  chore(withdrawer): replace contracts with `astria-bridge-contracts` submodule (#1164)
  feat(sequencer)!: implement bridge sudo and withdrawer addresses (#1142)
  feat(sequencer): implement refund to rollup logic upon ics20 transfer refund (#1161)
  feat(bridge-withdrawer): bridge withdrawer startup (#1160)
  feat(core, proto)!: add bech32m addresses (#1124)
  feat(withdrawer): bridged ERC20 token withdrawals (#1149)
  feat(sequencer-relayer)!: add chain IDs for sequencer and Celestia to config env vars (#1063)
  test(bridge-withdrawer): add submitter tests (#1133)
  chore: bump penumbra deps (#1159)
  feat(sequencer): implement `bridge/account_last_tx_hash` abci query (#1158)
  fix(withdrawer): use block subscription in batcher; send to destination_chain_address (#1157)
  fix(withdrawer): update AstriaWithdrawer to check that withdrawal value is sufficient (#1148)
  chore(ci): build bridge withdrawer images (#1156)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sequencer: Add logic for handling reversion of IBC-based rollup withdrawals
2 participants