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

If the storage.used increasing during the withdraw, the bridging transaction will be broken. #125

Closed
btspoony opened this issue Oct 1, 2024 · 2 comments · Fixed by #133
Assignees
Labels

Comments

@btspoony
Copy link
Contributor

btspoony commented Oct 1, 2024

Instructions

Bridge a NFT or FT which storage.used will be increated during the withdrawal.

Problem

The bridging to evm transaction will be in a error.

Steps to Reproduce

Transaction Sample: https://www.flowdiver.io/tx/0e84fe1f20b56b025647bf55135a38fdea40f371172128e2fc46d019c949b95f?tab=script

Panic line: https://github.com/onflow/flow-evm-bridge/blob/main/cadence/transactions/bridge/tokens/bridge_tokens_to_evm.cdc#L67

All Fixes FT will have a chance to add metadata to Vault during withdraw, so the storage.used will be increased.

Acceptance Criteria

Not to panic

Context

@sisyphusSmiling
Copy link
Contributor

Thanks for reporting. Yeah, we'll need to cover underflow in this case though it's not clear how the fee will be estimated in this case. The bridge has to charge fees based on storage consumption so there's a chance that the transaction will revert on withdrawing the fee from the scoped provider. I'll look into this

@sisyphusSmiling sisyphusSmiling self-assigned this Oct 1, 2024
@sisyphusSmiling sisyphusSmiling moved this to 🔖 Ready for Pickup in 🌊 Flow 4D Oct 1, 2024
@bluesign
Copy link

bluesign commented Oct 2, 2024

passing function for fee is one option. something like:

self.coa.depositTokens(
            vault: <-self.sentVault,
            feeProviderFunc: fun(amount: UFix64):@{FungibleToken.Vault}{
            //here you can assert fee and decide to bridge or not 
           ....
            }
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants