diff --git a/src/allocators/OnChainAllocator.sol b/src/allocators/OnChainAllocator.sol index d27f49a..015d0c4 100644 --- a/src/allocators/OnChainAllocator.sol +++ b/src/allocators/OnChainAllocator.sol @@ -535,24 +535,24 @@ contract OnChainAllocator is IOnChainAllocator { function _getAndUpdateNonce(address calling, address sponsor) internal returns (uint256 nonce) { assembly ("memory-safe") { - calling := xor(calling, mul(sponsor, iszero(calling))) - mstore(0x00, calling) + sponsor := mul(sponsor, iszero(calling)) + mstore(0x00, sponsor) mstore(0x20, nonces.slot) let nonceSlot := keccak256(0x00, 0x40) let nonce96 := sload(nonceSlot) - nonce := or(shl(96, calling), add(nonce96, 1)) + nonce := or(shl(96, sponsor), add(nonce96, 1)) sstore(nonceSlot, add(nonce96, 1)) } } function _getNonce(address calling, address sponsor) internal view returns (uint256 nonce) { assembly ("memory-safe") { - calling := xor(calling, mul(sponsor, iszero(calling))) - mstore(0x00, calling) + sponsor := mul(sponsor, iszero(calling)) + mstore(0x00, sponsor) mstore(0x20, nonces.slot) let nonceSlot := keccak256(0x00, 0x40) let nonce96 := sload(nonceSlot) - nonce := or(shl(96, calling), add(nonce96, 1)) + nonce := or(shl(96, sponsor), add(nonce96, 1)) } } diff --git a/test/ERC7683Allocator.t.sol b/test/ERC7683Allocator.t.sol index a1eb4f7..d281e69 100644 --- a/test/ERC7683Allocator.t.sol +++ b/test/ERC7683Allocator.t.sol @@ -1264,7 +1264,7 @@ contract ERC7683Allocator_openForDeposit is MockAllocator { assertEq(ERC6909(address(compactContract)).balanceOf(user, usdcId), defaultAmount); - compact_.nonce = _composeNonceUint(relayer, 1); + compact_.nonce = _composeNonceUint(address(0), 1); compact_.commitments[0].amount = defaultAmount; (bytes32 mandateHash,) = _hashMandate(mandate_); @@ -1281,7 +1281,7 @@ contract ERC7683Allocator_openForDeposit is MockAllocator { usdc.mint(address(erc7683Allocator), amount); BatchCompact memory compact_ = _getCompact(); - compact_.nonce = _composeNonceUint(relayer, 1); + compact_.nonce = _composeNonceUint(address(0), 1); Mandate memory mandate_ = _getMandate(); IOriginSettler.GaslessCrossChainOrder memory order_ = _getGaslessCrossChainOrder(compact_, mandate_, true); diff --git a/test/OnChainAllocator.t.sol b/test/OnChainAllocator.t.sol index da45976..d728d5d 100644 --- a/test/OnChainAllocator.t.sol +++ b/test/OnChainAllocator.t.sol @@ -984,9 +984,10 @@ contract OnChainAllocatorTest is Test, TestHelper { recipient, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' ); - assertEq(returnedNonce, _composeNonceUint(caller, 1)); + assertEq(returnedNonce, _composeNonceUint(address(0), 1)); // storage nonce is only incremented in executeAllocation assertEq(allocator.nonces(caller), 0); + assertEq(allocator.nonces(address(0)), 0); } function test_prepareAllocation_revert_InvalidExpiration() public { @@ -1036,8 +1037,9 @@ contract OnChainAllocatorTest is Test, TestHelper { vm.snapshotGasLastCall('onchain_execute_single'); // nonce is scoped to (callerContract, recipient) - assertEq(allocator.nonces(address(allocationCaller)), 1); - uint256 expectedNonce = _composeNonceUint(address(allocationCaller), 1); + assertEq(allocator.nonces(address(allocationCaller)), 0); + assertEq(allocator.nonces(address(0)), 1); + uint256 expectedNonce = _composeNonceUint(address(0), 1); // compute claim hash and check authorization Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts); @@ -1051,6 +1053,148 @@ contract OnChainAllocatorTest is Test, TestHelper { ); } + /// forge-config: default.isolate = false + function test_executeAllocation_revert_multiplePreparations() public { + uint256[2][] memory idsAndAmounts = _idsAndAmountsFor(address(usdc), defaultAmount); + + uint256[2][] memory idsAndAmountsCrooked = new uint256[2][](2); + idsAndAmountsCrooked[0][0] = _toId(Scope.Multichain, ResetPeriod.TenMinutes, address(allocator), address(0)); // additionally use another ids to receive a unique identifier slot + idsAndAmountsCrooked[0][1] = defaultAmount; + idsAndAmountsCrooked[1] = idsAndAmounts[0]; // Crooked idsAndAmounts is also using USDC as the same ID, among others + + usdc.mint(address(this), defaultAmount); + usdc.approve(address(compact), defaultAmount); + + // prepare for the recipient using caller 1 + vm.prank(recipient); + uint256 nonce1 = allocator.prepareAllocation( + recipient, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + // prepare for second recipient - DIFFERENT CALLER TO RECEIVE A DIFFERENT NONCE SLOT + vm.prank(address(this)); + uint256 nonce2 = allocator.prepareAllocation( + recipient, idsAndAmountsCrooked, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + assertEq(nonce1, _composeNonceUint(address(0), 1)); + assertEq(nonce2, _composeNonceUint(address(0), 1)); + + // We do a single deposit of usdc with defaultAmount. This would mean, that we SHOULD only able to allocate defaultAmount of usdc + ITheCompact(compact).batchDeposit{value: defaultAmount}(idsAndAmountsCrooked, recipient); + + // Crooked registrations: we register two claims that would each use all of the usdc (so defaultAmount * 2 combined) + Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts); + bytes32 claimHash1Crooked = + _createClaimHash(recipient, arbiter, nonce1, defaultExpiration, commitments, bytes32(0)); + Lock[] memory commitmentsCrooked = _idsAndAmountsToCommitments(idsAndAmountsCrooked); + bytes32 claimHash2Crooked = + _createClaimHash(recipient, arbiter, nonce2, defaultExpiration, commitmentsCrooked, bytes32(0)); + vm.prank(recipient); + ITheCompact(compact).register(claimHash1Crooked, BATCH_COMPACT_TYPEHASH); + vm.prank(recipient); + ITheCompact(compact).register(claimHash2Crooked, BATCH_COMPACT_TYPEHASH); + + // execute for first allocation + vm.prank(recipient); + allocator.executeAllocation( + recipient, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + // execute for second allocation which must fail + vm.prank(address(this)); + vm.expectRevert(abi.encodeWithSelector(AllocatorLib.InvalidPreparation.selector)); + allocator.executeAllocation( + recipient, idsAndAmountsCrooked, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + assertTrue( + allocator.isClaimAuthorized( + claimHash1Crooked, arbiter, recipient, nonce1, defaultExpiration, idsAndAmounts, '' + ) + ); + assertFalse( + allocator.isClaimAuthorized( + claimHash2Crooked, arbiter, recipient, nonce2, defaultExpiration, idsAndAmounts, '' + ) + ); + } + + /// forge-config: default.isolate = false + function test_executeAllocation_revert_multipleDifferentPreparations(address recipient1, address recipient2) + public + { + vm.assume(recipient1 != address(0)); + vm.assume(recipient2 != address(0)); + + uint256[2][] memory idsAndAmounts = _idsAndAmountsFor(address(usdc), defaultAmount); + + uint256 previousBalance = 1_000_000; + usdc.mint(address(this), previousBalance + 2 * defaultAmount); + usdc.approve(address(compact), previousBalance + 2 * defaultAmount); + + // deposit funds to recipient1 + compact.depositERC20( + address(usdc), + _toLockTag(address(allocator), Scope.Multichain, ResetPeriod.TenMinutes), + previousBalance, + recipient1 + ); + + // Check nonce previous to the allocation + assertEq(allocator.nonces(address(0)), 0); + + // prepare for first recipient + vm.prank(recipient1); + uint256 nonce1 = allocator.prepareAllocation( + recipient1, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + // prepare for second recipient + vm.prank(recipient2); + uint256 nonce2 = allocator.prepareAllocation( + recipient2, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + assertEq(nonce1, _composeNonceUint(address(0), 1)); + assertEq(nonce2, _composeNonceUint(address(0), 1)); + // The provided nonces should be the same because they use the same pool of nonces, independent of the recipient or caller + assertEq(nonce1, nonce2); + + // compute claim hash and check authorization + Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts); + bytes32 claimHash1 = _createClaimHash(recipient1, arbiter, nonce1, defaultExpiration, commitments, bytes32(0)); + bytes32 claimHash2 = _createClaimHash(recipient2, arbiter, nonce2, defaultExpiration, commitments, bytes32(0)); + + ITheCompact(compact).batchDepositAndRegisterFor( + recipient1, idsAndAmounts, arbiter, nonce1, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0) + ); + + ITheCompact(compact).batchDepositAndRegisterFor( + recipient2, idsAndAmounts, arbiter, nonce2, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0) + ); + + // execute for first recipient + vm.prank(recipient1); + allocator.executeAllocation( + recipient1, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + // execute for second recipient + vm.prank(recipient2); + vm.expectRevert(abi.encodeWithSelector(AllocatorLib.InvalidPreparation.selector)); + allocator.executeAllocation( + recipient2, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), '' + ); + + assertTrue( + allocator.isClaimAuthorized(claimHash1, arbiter, recipient1, nonce1, defaultExpiration, idsAndAmounts, '') + ); + assertFalse( + allocator.isClaimAuthorized(claimHash2, arbiter, recipient2, nonce2, defaultExpiration, idsAndAmounts, '') + ); + } + function test_executeAllocation_revert_InvalidPreparation() public { uint256 amount = defaultAmount; uint256[2][] memory idsAndAmounts = _idsAndAmountsFor(address(usdc), amount); @@ -1082,12 +1226,7 @@ contract OnChainAllocatorTest is Test, TestHelper { // Compute the claimHash that AllocatorLib will recompute during execute. Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts); bytes32 expectedClaimHash = _createClaimHash( - recipient, - arbiter, - _composeNonceUint(address(allocationCaller), 1), - defaultExpiration, - commitments, - bytes32(0) + recipient, arbiter, _composeNonceUint(address(0), 1), defaultExpiration, commitments, bytes32(0) ); vm.prank(user); vm.expectRevert( @@ -1143,7 +1282,7 @@ contract OnChainAllocatorTest is Test, TestHelper { vm.snapshotGasLastCall('onchain_execute_double'); // authorization with the measured amounts - uint256 expectedNonce = _composeNonceUint(address(allocationCaller), 1); + uint256 expectedNonce = _composeNonceUint(address(0), 1); assertTrue( allocator.isClaimAuthorized( @@ -1316,7 +1455,7 @@ contract OnChainAllocatorTest is Test, TestHelper { idsAndAmounts[0][0] = _toId(Scope.Multichain, ResetPeriod.TenMinutes, address(allocator), address(usdc)); idsAndAmounts[0][1] = defaultAmount; - assertEq(nonce, _composeNonceUint(caller, 1)); + assertEq(nonce, _composeNonceUint(address(0), 1)); assertEq(registeredAmounts.length, 1); assertEq(registeredAmounts[0], defaultAmount); assertEq(ERC6909(address(compact)).balanceOf(recipient, idsAndAmounts[0][0]), defaultAmount); @@ -1345,7 +1484,7 @@ contract OnChainAllocatorTest is Test, TestHelper { idsAndAmounts[0][0] = _toId(Scope.Multichain, ResetPeriod.TenMinutes, address(allocator), address(usdc)); idsAndAmounts[0][1] = defaultAmount; - assertEq(nonce, _composeNonceUint(caller, 1)); + assertEq(nonce, _composeNonceUint(address(0), 1)); assertEq(registeredAmounts.length, 1); assertEq(registeredAmounts[0], defaultAmount); assertEq(ERC6909(address(compact)).balanceOf(recipient, idsAndAmounts[0][0]), defaultAmount);