Bitcoin redeemer address validation #399
Conversation
Redemption data passed to the bridge are not validated, we enforce a check that the redeemer address is set to the token owner, so the funds are not locked accidentially in case of tBTC redemption timeout.
To test integration with the mainnet tBTC Bridge contracts for redemptions we add the integration test.
We update forking block number to match the block at which redemption test data can be used.
Introduce second depositor to test redemptions in the context of two depositors to better check the expected results for reverts and amounts. Introduce a test where a depositor wants to fully redeem their position.
✅ Deploy Preview for acre-dapp-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| tbtcRedemptionData, | ||
| (address, bytes20, bytes32, uint32, uint64, bytes) | ||
| ); | ||
| if (redeemer != owner) revert RedeemerNotOwner(redeemer, owner); |
There was a problem hiding this comment.
Most likely I don't have a full picture, but is it possible an owner give an approval for redeemer to redeem it's shares?
There was a problem hiding this comment.
The owner can set the redeemer to any address. If anything goes wrong with the tBTC redemption in tBTC Bridge, the redeemer address will receive the tBTC balance. In our case, we want to ensure the owner sets the redeemer address to themself, so they can recover the funds in case of a failure.
| @@ -58,6 +58,9 @@ contract BitcoinRedeemer is Ownable2StepUpgradeable, IReceiveApproval { | |||
| /// Attempted to call redeemSharesAndUnmint with unexpected tBTC token owner. | |||
| error UnexpectedTbtcTokenOwner(); | |||
|
|
|||
| /// Reverts if the redeemer address is zero. | |||
There was a problem hiding this comment.
It would be enough that a redeemer is not an owner, not necessarily a zero address.
| tbtcRedemptionData, | ||
| (address, bytes20, bytes32, uint32, uint64, bytes) | ||
| ); | ||
| if (redeemer != owner) revert RedeemerNotOwner(redeemer, owner); |
There was a problem hiding this comment.
Can we maybe move the validation above stbtc.redeem(...) to save on gas in case of a revert?
The
redeemeraddress passed to the tBTC Bridge in the redemption data was not validated. In this PR we enforce a check that the redeemer address is set to the token owner, so the fundsare not locked accidentally in case of tBTC redemption timeout.