Skip to content

Commit

Permalink
Simplify address checks in receiveApproval function (#405)
Browse files Browse the repository at this point in the history
We can check directly msg.sender agains stbtc address and skip checking
the token address, as it is expected that the function will be called
only by the stBTC contract, and the token argument is not used anywhere,
besides the validation.
  • Loading branch information
dimpar authored May 8, 2024
2 parents 34015e9 + 9b8d6aa commit 3cbd753
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 10 deletions.
9 changes: 2 additions & 7 deletions solidity/contracts/BitcoinRedeemer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ contract BitcoinRedeemer is Ownable2StepUpgradeable, IReceiveApproval {
/// Reverts if the TBTCVault address is zero.
error TbtcVaultZeroAddress();

/// Attempted to call receiveApproval for not supported token.
error UnsupportedToken(address token);

/// Attempted to call receiveApproval by supported token.
error CallerNotAllowed(address caller);

Expand Down Expand Up @@ -96,18 +93,16 @@ contract BitcoinRedeemer is Ownable2StepUpgradeable, IReceiveApproval {
/// @notice Redeems shares for tBTC and requests bridging to Bitcoin.
/// @param from Shares token holder executing redemption.
/// @param amount Amount of shares to redeem.
/// @param token stBTC token address.
/// @param extraData Redemption data in a format expected from
/// `redemptionData` parameter of Bridge's `receiveBalanceApproval`
/// function.
function receiveApproval(
address from,
uint256 amount,
address token,
address,
bytes calldata extraData
) external {
if (token != address(stbtc)) revert UnsupportedToken(token);
if (msg.sender != token) revert CallerNotAllowed(msg.sender);
if (msg.sender != address(stbtc)) revert CallerNotAllowed(msg.sender);
if (extraData.length == 0) revert EmptyExtraData();

redeemSharesAndUnmint(from, amount, extraData);
Expand Down
4 changes: 2 additions & 2 deletions solidity/test/BitcoinDepositor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ describe("BitcoinDepositor", () => {
extraDataValidTestData.forEach(
(
{
depositOwner: expoectedDepositOwner,
depositOwner: expectedDepositOwner,
referral: expectedReferral,
extraData,
},
Expand All @@ -759,7 +759,7 @@ describe("BitcoinDepositor", () => {
await bitcoinDepositor.decodeExtraData(extraData)

expect(actualDepositOwner, "invalid depositOwner").to.be.equal(
expoectedDepositOwner,
expectedDepositOwner,
)
expect(actualReferral, "invalid referral").to.be.equal(
expectedReferral,
Expand Down
2 changes: 1 addition & 1 deletion solidity/test/BitcoinRedeemer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe("BitcoinRedeemer", () => {
depositor.address,
encodeBytes32String(""),
),
).to.be.revertedWithCustomError(bitcoinRedeemer, "UnsupportedToken")
).to.be.revertedWithCustomError(bitcoinRedeemer, "CallerNotAllowed")
})
})

Expand Down

0 comments on commit 3cbd753

Please sign in to comment.