From 079175429f412d068f9372493f3e8ac438ec08c7 Mon Sep 17 00:00:00 2001 From: "John McClure (pickleback)" Date: Thu, 19 Dec 2024 01:22:03 -0600 Subject: [PATCH 1/3] add logic for reading/writing more than 32 bytes into the `TendConfig.extraData` field --- contracts/EverlongStrategy.sol | 49 ++++++++++++++++++++++++++++++++-- test/everlong/units/Tend.t.sol | 37 +++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/contracts/EverlongStrategy.sol b/contracts/EverlongStrategy.sol index 687f874..2645e0d 100644 --- a/contracts/EverlongStrategy.sol +++ b/contracts/EverlongStrategy.sol @@ -375,7 +375,27 @@ contract EverlongStrategy is BaseStrategy { tstore(minOutputSlot, minOutput) tstore(minVaultSharePriceSlot, minVaultSharePrice) tstore(positionClosureLimitSlot, positionClosureLimit) - tstore(extraDataSlot, extraData) + + // Load the length of `extraData`. + let extraDataLength := mload(extraData) + + // Pointer to the actual data. + let extraDataData := add(extraData, 0x20) + + // Store the length in the `extraDataSlot`. + tstore(extraDataSlot, extraDataLength) + + // Store the data in subsequent slots. + for { + let i := 0 + } lt(i, extraDataLength) { + i := add(i, 32) + } { + tstore( + add(extraDataSlot, add(div(i, 32), 1)), + mload(add(extraDataData, i)) + ) + } } } @@ -406,7 +426,32 @@ contract EverlongStrategy is BaseStrategy { minOutput := tload(minOutputSlot) minVaultSharePrice := tload(minVaultSharePriceSlot) positionClosureLimit := tload(positionClosureLimitSlot) - extraData := tload(extraDataSlot) + + // Load the length of `extraData`. + let extraDataLength := tload(extraDataSlot) + + // Get free memory pointer. + let extraDataData := mload(0x40) + + // Allocate memory. + mstore(0x40, add(extraDataData, add(extraDataLength, 0x20))) + + // Store the length. + mstore(extraDataData, extraDataLength) + + // Load the data from transient storage. + for { + let i := 0 + } lt(i, extraDataLength) { + i := add(i, 32) + } { + mstore( + add(extraDataData, add(0x20, i)), + tload(add(extraDataSlot, add(div(i, 32), 1))) + ) + } + + extraData := extraDataData } // Return the TendConfig. diff --git a/test/everlong/units/Tend.t.sol b/test/everlong/units/Tend.t.sol index 175bf77..1dfdf2e 100644 --- a/test/everlong/units/Tend.t.sol +++ b/test/everlong/units/Tend.t.sol @@ -295,4 +295,41 @@ contract TestTend is EverlongTest { // Stop the prank. vm.stopPrank(); } + + /// @dev Tests that `TendConfig.extraData` is fully loaded even when the + /// content is greater than a single storage slot. + function test_extraData_large() external { + // Start a prank as the keeper address. + vm.startPrank(address(keeperContract)); + + // Prepare a large `bytes` array for `extraData`. + bytes memory largeExtraData = new bytes(1024); + for (uint256 i = 0; i < 1024; i++) { + largeExtraData[i] = bytes1(uint8(i % 256)); + } + + // Call `setTendConfig` with the large `extraData`. + IEverlongStrategy(address(strategy)).setTendConfig( + IEverlongStrategy.TendConfig({ + minOutput: 1, + minVaultSharePrice: 0, + positionClosureLimit: 0, + extraData: largeExtraData + }) + ); + + // Retrieve the TendConfig. + (, IEverlongStrategy.TendConfig memory tendConfig) = IEverlongStrategy( + address(strategy) + ).getTendConfig(); + + // Assert that the retrieved `extraData` is the same as the input. + assertTrue( + keccak256(tendConfig.extraData) == keccak256(largeExtraData), + "Large bytes data mismatch!" + ); + + // Stop the prank. + vm.stopPrank(); + } } From b9d9ebbd9c28c14501ac9df994c8b1426a698b10 Mon Sep 17 00:00:00 2001 From: Xiangyu Xu Date: Wed, 19 Mar 2025 20:24:16 -0700 Subject: [PATCH 2/3] Resolve @jalextowle 's comment --- contracts/EverlongStrategy.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/EverlongStrategy.sol b/contracts/EverlongStrategy.sol index 2645e0d..d1c651e 100644 --- a/contracts/EverlongStrategy.sol +++ b/contracts/EverlongStrategy.sol @@ -371,7 +371,7 @@ contract EverlongStrategy is BaseStrategy { uint256 positionClosureLimit = _config.positionClosureLimit; bytes memory extraData = _config.extraData; assembly { - tstore(tendEnabledSlot, 1) + tstore(tendEnabledSlot, 0x1) tstore(minOutputSlot, minOutput) tstore(minVaultSharePriceSlot, minVaultSharePrice) tstore(positionClosureLimitSlot, positionClosureLimit) @@ -389,10 +389,10 @@ contract EverlongStrategy is BaseStrategy { for { let i := 0 } lt(i, extraDataLength) { - i := add(i, 32) + i := add(i, 0x20) } { tstore( - add(extraDataSlot, add(div(i, 32), 1)), + add(extraDataSlot, add(div(i, 0x20), 0x1)), mload(add(extraDataData, i)) ) } @@ -443,11 +443,11 @@ contract EverlongStrategy is BaseStrategy { for { let i := 0 } lt(i, extraDataLength) { - i := add(i, 32) + i := add(i, 0x20) } { mstore( add(extraDataData, add(0x20, i)), - tload(add(extraDataSlot, add(div(i, 32), 1))) + tload(add(extraDataSlot, add(div(i, 0x20), 0x1))) ) } From bf77a019720f2c8c8025c35db60ea97f4acab758 Mon Sep 17 00:00:00 2001 From: Xiangyu Xu Date: Wed, 19 Mar 2025 21:01:49 -0700 Subject: [PATCH 3/3] Added tests to resolve the auditors' comments --- test/everlong/units/Tend.t.sol | 166 +++++++++++++++++++++++++++++++-- 1 file changed, 160 insertions(+), 6 deletions(-) diff --git a/test/everlong/units/Tend.t.sol b/test/everlong/units/Tend.t.sol index 1dfdf2e..eac959f 100644 --- a/test/everlong/units/Tend.t.sol +++ b/test/everlong/units/Tend.t.sol @@ -312,8 +312,8 @@ contract TestTend is EverlongTest { IEverlongStrategy(address(strategy)).setTendConfig( IEverlongStrategy.TendConfig({ minOutput: 1, - minVaultSharePrice: 0, - positionClosureLimit: 0, + minVaultSharePrice: 2, + positionClosureLimit: 3, extraData: largeExtraData }) ); @@ -324,12 +324,166 @@ contract TestTend is EverlongTest { ).getTendConfig(); // Assert that the retrieved `extraData` is the same as the input. - assertTrue( - keccak256(tendConfig.extraData) == keccak256(largeExtraData), - "Large bytes data mismatch!" + assertEq(tendConfig.minOutput, 1, "minOutput mismatch"); + assertEq(tendConfig.minVaultSharePrice, 2, "minVaultSharePrice mismatch"); + assertEq(tendConfig.positionClosureLimit, 3, "positionClosureLimit mismatch"); + + // Use bytes comparison for extraData + assertEq(tendConfig.extraData.length, largeExtraData.length, "extraData length mismatch"); + + for (uint256 i = 0; i < largeExtraData.length; i++) { + assertEq(uint8(tendConfig.extraData[i]), uint8(largeExtraData[i]), string(abi.encodePacked("extraData byte mismatch at index ", i))); + } + + // Stop the prank. + vm.stopPrank(); + } + + /// @dev Tests that empty `extraData` is correctly handled. + function test_extraData_empty() external { + // Start a prank as the keeper address. + vm.startPrank(address(keeperContract)); + + // Empty extraData + bytes memory emptyData = new bytes(0); + + // Set TendConfig with empty extraData + IEverlongStrategy(address(strategy)).setTendConfig( + IEverlongStrategy.TendConfig({ + minOutput: 42, + minVaultSharePrice: 100, + positionClosureLimit: 5, + extraData: emptyData + }) ); + // Retrieve the TendConfig + (, IEverlongStrategy.TendConfig memory tendConfig) = IEverlongStrategy( + address(strategy) + ).getTendConfig(); + + // Assert all fields + assertEq(tendConfig.minOutput, 42, "minOutput mismatch"); + assertEq(tendConfig.minVaultSharePrice, 100, "minVaultSharePrice mismatch"); + assertEq(tendConfig.positionClosureLimit, 5, "positionClosureLimit mismatch"); + assertEq(tendConfig.extraData.length, 0, "extraData should be empty"); + + // Stop the prank. + vm.stopPrank(); + } + + /// @dev Tests that `extraData` smaller than 32 bytes is correctly handled. + function test_extraData_small() external { + // Start a prank as the keeper address. + vm.startPrank(address(keeperContract)); + + // Small extraData (<32 bytes) + bytes memory smallData = bytes("small data"); + + // Set TendConfig with small extraData + IEverlongStrategy(address(strategy)).setTendConfig( + IEverlongStrategy.TendConfig({ + minOutput: 123, + minVaultSharePrice: 456, + positionClosureLimit: 789, + extraData: smallData + }) + ); + + // Retrieve the TendConfig + (, IEverlongStrategy.TendConfig memory tendConfig) = IEverlongStrategy( + address(strategy) + ).getTendConfig(); + + // Assert all fields + assertEq(tendConfig.minOutput, 123, "minOutput mismatch"); + assertEq(tendConfig.minVaultSharePrice, 456, "minVaultSharePrice mismatch"); + assertEq(tendConfig.positionClosureLimit, 789, "positionClosureLimit mismatch"); + + // Check extraData + assertEq(tendConfig.extraData.length, smallData.length, "extraData length mismatch"); + assertEq(keccak256(tendConfig.extraData), keccak256(smallData), "extraData content mismatch"); + + // Stop the prank. + vm.stopPrank(); + } + + /// @dev Tests that `extraData` exactly 32 bytes is correctly handled. + function test_extraData_exactly32Bytes() external { + // Start a prank as the keeper address. + vm.startPrank(address(keeperContract)); + + // Create exactly 32 bytes of data + bytes memory data32 = new bytes(32); + for (uint256 i = 0; i < 32; i++) { + data32[i] = bytes1(uint8(i + 1)); + } + + // Set TendConfig with 32-byte extraData + IEverlongStrategy(address(strategy)).setTendConfig( + IEverlongStrategy.TendConfig({ + minOutput: 0xdead, + minVaultSharePrice: 0xbeef, + positionClosureLimit: 0xcafe, + extraData: data32 + }) + ); + + // Retrieve the TendConfig + (, IEverlongStrategy.TendConfig memory tendConfig) = IEverlongStrategy( + address(strategy) + ).getTendConfig(); + + // Assert all fields + assertEq(tendConfig.minOutput, 0xdead, "minOutput mismatch"); + assertEq(tendConfig.minVaultSharePrice, 0xbeef, "minVaultSharePrice mismatch"); + assertEq(tendConfig.positionClosureLimit, 0xcafe, "positionClosureLimit mismatch"); + + // Check extraData + assertEq(tendConfig.extraData.length, data32.length, "extraData length mismatch"); + + for (uint256 i = 0; i < data32.length; i++) { + assertEq(uint8(tendConfig.extraData[i]), uint8(data32[i]), string(abi.encodePacked("extraData byte mismatch at index ", i))); + } + + // Stop the prank. + vm.stopPrank(); + } + + /// @dev Tests that `extraData` with a single byte is correctly handled. + function test_extraData_singleByte() external { + // Start a prank as the keeper address. + vm.startPrank(address(keeperContract)); + + // Single byte extraData + bytes memory singleByteData = new bytes(1); + singleByteData[0] = 0x42; + + // Set TendConfig with single byte extraData + IEverlongStrategy(address(strategy)).setTendConfig( + IEverlongStrategy.TendConfig({ + minOutput: 999, + minVaultSharePrice: 888, + positionClosureLimit: 777, + extraData: singleByteData + }) + ); + + // Retrieve the TendConfig + (, IEverlongStrategy.TendConfig memory tendConfig) = IEverlongStrategy( + address(strategy) + ).getTendConfig(); + + // Assert all fields + assertEq(tendConfig.minOutput, 999, "minOutput mismatch"); + assertEq(tendConfig.minVaultSharePrice, 888, "minVaultSharePrice mismatch"); + assertEq(tendConfig.positionClosureLimit, 777, "positionClosureLimit mismatch"); + + // Check extraData + assertEq(tendConfig.extraData.length, 1, "extraData length should be 1"); + assertEq(uint8(tendConfig.extraData[0]), 0x42, "extraData byte value mismatch"); + // Stop the prank. vm.stopPrank(); } -} +} \ No newline at end of file