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

fix(withdrawer): update AstriaWithdrawer to check that withdrawal value is sufficient #1148

Merged
merged 9 commits into from
Jun 5, 2024

Conversation

noot
Copy link
Collaborator

@noot noot commented Jun 5, 2024

Summary

update AstriaWithdrawer to check that withdrawal value is sufficient. for example, say a bridged asset is multiplied by 10^9 upon bridging to the rollup from the sequencer. you should not be able to withdraw some value <10^9, as this is not a valid asset amount on the sequencer, as it's smaller than the smallest denomination on the sequencer. this updates the contract to perform that check. the alternative is to update the withdrawer not to send sequencer withdrawals with value=0, but this method is better for ux as it won't allow a user to make an invalid withdrawal in the first place.

Background

found while debugging

Changes

  • update AstriaWithdrawer to check that withdrawal value is sufficient, ie. value >= 10^assetWithdrawalDecimals

Testing

unit test

@noot noot requested review from a team, joroshiba and SuperFluffy as code owners June 5, 2024 02:03
@noot noot requested a review from a team as a code owner June 5, 2024 02:10
@noot noot added this pull request to the merge queue Jun 5, 2024
Merged via the queue into main with commit b62563d Jun 5, 2024
37 checks passed
@noot noot deleted the noot/withdrawer-decimals-check branch June 5, 2024 19:29
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants