bnb-chain / opbnb

MIT License
403 stars 162 forks source link

Optimism SDK withdrawl proof fails #213

Closed PlasticDigits closed 2 months ago

PlasticDigits commented 2 months ago

System information

Running on BSC/opBNB mainnet (56, 204)

Expected behaviour

Proofs for L2 BNB and token withdraws should be calculated and submitted to BSC L1.

Actual behaviour

Theres a bug in the CrossChainMessenger getMessageBedrockOutput. BSC <> opBNB needs the most recent output index, but the optimism sdk uses the most recent index after the transaction. I have built a template dapp for BSC <> opBNB bridges with custom tokens, it fixes this bug by overriding getMesageBedrockOutput. The dapp is live at https://microscopic-continent-rhythmic.on-fleek.app/ and contains the fix. The broken code in Optimism SDK is at https://github.com/ethereum-optimism/optimism/blob/ee058e413e0c7ff92d000593af648f3d79ca1a31/packages/sdk/src/cross-chain-messenger.ts#L1076.

You can see the working code fix in my template dapp https://github.com/chinese-zodiac/bridge-opbnb-cz-cash-fe/blob/main/src/components/account/WithdrawAccount.js#L94

owen-reorg commented 2 months ago

Thanks for your information!

This is not a bug; both proofs work for the withdrawal. The reason we use the most recent output index on BSC is that BSC has more historical state data than Ethereum, making it harder to preserve and retrieve the proof of an older block.

The @eth-optimism/sdk has been replaced with op-viem. op-viem uses a new interface for the withdrawal process, breaking down the withdrawal process. This allows us to choose to use the proof of the latest index or the next index after the initWithdrawal as desired.

There is a proof of concept available as a reference: https://gist.github.com/owen-reorg/568ae7e5ae24df1db2ef2f58a623621b