From d80cc99eea1606647ef9047f7257e1ecdaf1b0dd Mon Sep 17 00:00:00 2001 From: Lawrence Forman Date: Fri, 8 Mar 2024 19:55:04 -0500 Subject: [PATCH] return paid amount from claims --- script/demo/Testnet.s.sol | 3 +- src/HoneyPause.sol | 41 ++++++++++++++++++------- src/LibSandbox.sol | 2 +- test/HoneyPause.t.sol | 64 +++++++++++++++++++++++++++------------ 4 files changed, 78 insertions(+), 32 deletions(-) diff --git a/script/demo/Testnet.s.sol b/script/demo/Testnet.s.sol index a56ab07..1532466 100644 --- a/script/demo/Testnet.s.sol +++ b/script/demo/Testnet.s.sol @@ -72,7 +72,8 @@ contract Testnet is Script { hash, operator ); - require(honey.verifyBountyCanPay(d.bountyId(), payable(tx.origin)), 'cannot pay'); + (bool canPay,) = honey.verifyBountyCanPay(d.bountyId(), payable(tx.origin)); + require(canPay, 'cannot pay'); } function _broadcast() private { diff --git a/src/HoneyPause.sol b/src/HoneyPause.sol index a48de3c..9ca3e24 100644 --- a/src/HoneyPause.sol +++ b/src/HoneyPause.sol @@ -204,9 +204,10 @@ contract HoneyPause { bytes memory exploiterData, bytes memory verifierData ) - external + external returns (uint256 payAmount) { - Bounty memory bounty = _claim({ + Bounty memory bounty; + (bounty, payAmount) = _claim({ bountyId: bountyId, payReceiver: payReceiver, exploiter: exploiter, @@ -214,7 +215,7 @@ contract HoneyPause { verifierData: verifierData, skipExploit: false }); - emit Claimed(bountyId, bounty.payoutToken, bounty.payoutAmount); + emit Claimed(bountyId, bounty.payoutToken, payAmount); } /// @notice Carries out an exploit, verifies it, then reverts the call frame. @@ -243,7 +244,7 @@ contract HoneyPause { verifier.assertExploit, (verifierData, verifierStateData) )); - revert SandboxSucceededError(); + revert SandboxSucceededError(''); } /// @notice Check whether a bounty can actually pay out its reward. @@ -251,15 +252,24 @@ contract HoneyPause { /// determination. All state is reverted before returning so this can /// be safely called on-chain but most likely it will be consumed off-chain /// via eth_call. + /// @param bountyId ID of a valid, active bounty. + /// @param payReceiver Recepient of bounty. + /// @return bountyCanPay Whether the payer sent at least the bounty amount to the receiver. + /// @return payAmount Actual amount sent to receiver (may be more than bounty amount). function verifyBountyCanPay(uint256 bountyId, address payable payReceiver) - external returns (bool bountyCanPay) + external returns (bool bountyCanPay, uint256 payAmount) { try this.sandboxTryPayBounty(bountyId, payReceiver) { // Should always fail. assert(false); } catch (bytes memory errData) { - return LibSandbox.handleSandboxCallRevert(errData, true); + if (!LibSandbox.handleSandboxCallRevert(errData, true)) { + return (false, 0); + } + // The data inside the SandboxSucceededError is the pay amount. + payAmount = abi.decode(abi.decode(errData.skip(4), (bytes)), (uint256)); } + return (true, payAmount); } /// @notice Mimics the logic for a successful claim() to verify that a bounty can @@ -269,7 +279,7 @@ contract HoneyPause { function sandboxTryPayBounty(uint256 bountyId, address payable payReceiver) external { - _claim({ + (, uint256 payAmount) = _claim({ bountyId: bountyId, payReceiver: payReceiver, exploiter: IExploiter(address(0)), @@ -279,7 +289,7 @@ contract HoneyPause { // it does minus the exploit verification. skipExploit: true }); - revert SandboxSucceededError(); + revert SandboxSucceededError(abi.encode(payAmount)); } /// @dev The logic of claim(), refactored out for sandboxTryPayBounty(). @@ -292,7 +302,7 @@ contract HoneyPause { bool skipExploit ) internal - returns (Bounty memory bounty) + returns (Bounty memory bounty, uint256 payAmount) { bounty = getBounty[bountyId]; if (bounty.operator == address(0)) { @@ -318,11 +328,18 @@ contract HoneyPause { // Pause the protocol. address(bounty.pauser).safeCall(abi.encodeCall(bounty.pauser.pause, (bountyId))); // Pay the bounty. - _payout(bountyId, bounty.payer, bounty.payoutToken, payReceiver, bounty.payoutAmount); + payAmount = _payout( + bountyId, + bounty.payer, + bounty.payoutToken, + payReceiver, + bounty.payoutAmount + ); } - // Call a bounty's payer contract and verify that it transferred the payment. + // Call a bounty's payer contract, verify that it transferred the payment, and + // return the amount transferred. function _payout( uint256 bountyId, IPayer payer, @@ -331,6 +348,7 @@ contract HoneyPause { uint256 amount ) internal + returns (uint256 payAmount) { uint256 balBefore = _balanceOf(token, to); address(payer).safeCall(abi.encodeCall( @@ -341,6 +359,7 @@ contract HoneyPause { if (balBefore > balAfter || balAfter - balBefore < amount) { revert InsufficientPayoutError(); } + return balAfter - balBefore; } // Get the balance of a token (or ETH) of an account. diff --git a/src/LibSandbox.sol b/src/LibSandbox.sol index d8f28a6..12367ad 100644 --- a/src/LibSandbox.sol +++ b/src/LibSandbox.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.24; import { LibBytes } from './LibBytes.sol'; error SandboxFailedError(bytes innerError); -error SandboxSucceededError(); +error SandboxSucceededError(bytes data); library LibSandbox { using LibBytes for bytes; diff --git a/test/HoneyPause.t.sol b/test/HoneyPause.t.sol index 5fbef03..9e0bf80 100644 --- a/test/HoneyPause.t.sol +++ b/test/HoneyPause.t.sol @@ -38,9 +38,9 @@ contract HoneyPauseTest is Test { TestHoneyPause honey = new TestHoneyPause(); TestERC20 testToken = new TestERC20(); bytes TEST_ERROR = abi.encodeWithSelector(TestError.selector, 'FAILED'); - bytes SANDBOX_SUCCEEDED_ERROR = abi.encodeWithSelector(SandboxSucceededError.selector); + bytes EMPTY_SANDBOX_SUCCEEDED_ERROR = abi.encodeWithSelector(SandboxSucceededError.selector, ""); bytes WRAPPED_SANDBOX_SUCCEEDED_ERROR = abi.encodeWithSelector(SandboxFailedError.selector, - abi.encodeWithSelector(SandboxSucceededError.selector) + abi.encodeWithSelector(SandboxSucceededError.selector, "") ); function test_errorSelectorsCanBeSandboxed() external { @@ -99,7 +99,7 @@ contract HoneyPauseTest is Test { function test_sandboxExploit_revertsOnSuccess() external { IVerifier verifier = new TestVerifier(); IExploiter exploiter = new TestExploiter(); - vm.expectRevert(SandboxSucceededError.selector); + vm.expectRevert(EMPTY_SANDBOX_SUCCEEDED_ERROR); honey.sandboxExploit(exploiter, verifier, "", ""); } @@ -136,7 +136,7 @@ contract HoneyPauseTest is Test { function test_sandboxExploit_wrapsSandboxSuccessFromVerifierBeforeExploitCall() external { IVerifier verifier = IVerifier(_createFailingFnContract( IVerifier.beforeExploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); IExploiter exploiter = new TestExploiter(); vm.expectRevert(WRAPPED_SANDBOX_SUCCEEDED_ERROR); @@ -146,7 +146,7 @@ contract HoneyPauseTest is Test { function test_sandboxExploit_wrapsSandboxSuccessFromVerifierAssertExploitCall() external { IVerifier verifier = IVerifier(_createFailingFnContract( IVerifier.assertExploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); TestExploiter exploiter = new TestExploiter(); vm.expectRevert(WRAPPED_SANDBOX_SUCCEEDED_ERROR); @@ -157,7 +157,7 @@ contract HoneyPauseTest is Test { IVerifier verifier = new TestVerifier(); IExploiter exploiter = IExploiter(_createFailingFnContract( IExploiter.exploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); vm.expectRevert(WRAPPED_SANDBOX_SUCCEEDED_ERROR); honey.sandboxExploit(exploiter, verifier, "", ""); @@ -176,7 +176,7 @@ contract HoneyPauseTest is Test { emit TestExploiter.ExploitCalled(exploiterData); vm.expectEmit(true, true, true, true); emit TestVerifier.AssertExploitCalled(verifierData, verifierStateData); - vm.expectRevert(SandboxSucceededError.selector); + vm.expectRevert(EMPTY_SANDBOX_SUCCEEDED_ERROR); honey.sandboxExploit(exploiter, verifier, exploiterData, verifierData); } @@ -349,8 +349,9 @@ contract HoneyPauseTest is Test { IExploiter exploiter = new TestExploiter(); vm.expectEmit(true, true, true, true); emit Claimed(bountyId, ETH_TOKEN, 100); - honey.claim(bountyId, RECEIVER, exploiter, "", ""); + uint256 payAmount = honey.claim(bountyId, RECEIVER, exploiter, "", ""); assertEq(honey.isBountyClaimed(bountyId), true); + assertEq(payAmount, TEST_BOUNTY_AMOUNT); } function test_claim_passesData() external { @@ -431,15 +432,26 @@ contract HoneyPauseTest is Test { uint256 bountyId = _addTestBounty(); IExploiter exploiter = IExploiter(_createFailingFnContract( IExploiter.exploit.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR )); - vm.expectRevert(SANDBOX_SUCCEEDED_ERROR); + vm.expectRevert(EMPTY_SANDBOX_SUCCEEDED_ERROR); honey.claim(bountyId, RECEIVER, exploiter, "", ""); } function test_verifyBountyCanPay_returnsTrueIfBountyPays() external { uint256 bountyId = _addTestBounty(); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), true); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, true); + assertEq(payAmount, TEST_BOUNTY_AMOUNT); + } + + function test_verifyBountyCanPay_returnsTrueIfBountyPaysTooMuch() external { + uint256 bountyId = _addTestBounty(); + (,,,,, IPayer payer) = honey.getBounty(bountyId); + payable(address(payer)).transfer(1); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, true); + assertEq(payAmount, TEST_BOUNTY_AMOUNT + 1); } function test_verifyBountyCanPay_doesNotModifyState() external { @@ -457,7 +469,9 @@ contract HoneyPauseTest is Test { IPayer.payExploiter.selector, TEST_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPauserReverts() external { @@ -466,37 +480,47 @@ contract HoneyPauseTest is Test { IPauser.pause.selector, TEST_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPayerDoesNotPayEnoughEth() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPayer(bountyId, new TestEthPayer{value: TEST_BOUNTY_AMOUNT - 1}()); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPayerDoesNotPayEnoughErc20() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPayer(bountyId, new TestERC20Payer(testToken, TEST_BOUNTY_AMOUNT - 1)); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPayerRevertsWithSandoxSucceeded() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPayer(bountyId, IPayer(_createFailingFnContract( IPayer.payExploiter.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function test_verifyBountyCanPay_returnsFalseIfPauserRevertsWithSandoxSucceeded() external { uint256 bountyId = _addTestBounty(); honey.__testSetBountyPauser(bountyId, IPauser(_createFailingFnContract( IPauser.pause.selector, - SANDBOX_SUCCEEDED_ERROR + EMPTY_SANDBOX_SUCCEEDED_ERROR ))); - assertEq(honey.verifyBountyCanPay(bountyId, RECEIVER), false); + (bool canPay, uint256 payAmount) = honey.verifyBountyCanPay(bountyId, RECEIVER); + assertEq(canPay, false); + assertEq(payAmount, 0); } function _addTestBounty() @@ -632,6 +656,8 @@ contract TestEthPayer is IPayer { assert(token == ETH_TOKEN); return to.transfer(address(this).balance); } + + receive() external payable {} } contract TestHoneyPause is HoneyPause {