From c787fd6b34c2c621febca205390b9fe985eb88d3 Mon Sep 17 00:00:00 2001 From: Harsh Pandey Date: Thu, 26 Feb 2026 23:10:42 +0530 Subject: [PATCH 1/5] feat: add validation that payload has no storage vars --- foundry.toml | 4 ++- src/ProtocolV3TestBase.sol | 48 ++++++++++++++++++++++++++++++ tests/ProtocolV3TestBase.t.sol | 21 +++++++++++++ tests/mocks/PayloadWithStorage.sol | 16 ++++++++++ 4 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 tests/mocks/PayloadWithStorage.sol diff --git a/foundry.toml b/foundry.toml index 6d3c4ce41..80f85e121 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,10 +5,12 @@ script = 'scripts' out = 'out' libs = ['lib'] remappings = [] -fs_permissions = [{ access = "read-write", path = "./reports" }] +fs_permissions = [{ access = "read-write", path = "./reports" }, { access = "read", path = "./out" }] ffi = true evm_version = 'cancun' decode_external_storage = true +allow_internal_expect_revert = true +extra_output = ["storageLayout"] [profile.zksync] src = 'zksync/src' diff --git a/src/ProtocolV3TestBase.sol b/src/ProtocolV3TestBase.sol index 8ef59c260..d3151f53a 100644 --- a/src/ProtocolV3TestBase.sol +++ b/src/ProtocolV3TestBase.sol @@ -94,6 +94,9 @@ contract ProtocolV3TestBase is RawProtocolV3TestBase, SeatbeltUtils, CommonTestB uint256 gasUsed = startGas - gasleft(); assertLt(gasUsed, (block.gaslimit * 95) / 100, 'BLOCK_GAS_LIMIT_EXCEEDED'); // 5% is kept as a buffer + // as executor does delegateCall to the payload, the payload should have no storage variable + _validateNoPayloadStorageSlots(payload); + ReserveConfig[] memory configAfter = createConfigurationSnapshot(afterString, pool); { @@ -729,4 +732,49 @@ contract ProtocolV3TestBase is RawProtocolV3TestBase, SeatbeltUtils, CommonTestB vm.stopPrank(); } + + /** + * @dev Validates that a payload contract declares no state variables by inspecting the + * compiler-generated storage layout from the build artifact. + * + * If the artifact cannot be resolved (e.g. the contract was not compiled locally), a + * warning is logged and the check is skipped rather than failing the test. + * + * Requires foundry.toml to have: + * extra_output = ["storageLayout"] + * fs_permissions includes { access = "read", path = "./out" } + */ + function _validateNoPayloadStorageSlots(address payload) internal view { + string memory artifactPath; + try vm.getArtifactPathByDeployedCode(payload.code) returns (string memory path) { + artifactPath = path; + } catch { + console.log( + 'WARNING: _validateNoPayloadStorageSlots: could not resolve artifact for payload %s, skipping storage slot check', + payload + ); + return; + } + + string memory artifact = vm.readFile(artifactPath); + + // vm.parseJson ABI-encodes the JSON value at the given key. + // A dynamic array is encoded as [uint256 offset][uint256 length][elements...], so + // decoding the first two words as (uint256, uint256) yields (offset, arrayLength). + bytes memory storageEncoded = vm.parseJson(artifact, '.storageLayout.storage'); + (, uint256 storageLength) = abi.decode(storageEncoded, (uint256, uint256)); + + require( + storageLength == 0, + string( + abi.encodePacked( + 'PAYLOAD_MUST_NOT_HAVE_STORAGE_VARIABLES: ', + artifactPath, + ' declares ', + Strings.toString(storageLength), + ' storage slot(s)' + ) + ) + ); + } } diff --git a/tests/ProtocolV3TestBase.t.sol b/tests/ProtocolV3TestBase.t.sol index e005b3d91..6c64b285f 100644 --- a/tests/ProtocolV3TestBase.t.sol +++ b/tests/ProtocolV3TestBase.t.sol @@ -13,6 +13,7 @@ import {AaveV3MegaEth} from 'aave-address-book/AaveV3MegaEth.sol'; import {AaveV3Mantle} from 'aave-address-book/AaveV3Mantle.sol'; import {AaveV3Fantom} from 'aave-address-book/AaveV3Fantom.sol'; import {PayloadWithEmit} from './mocks/PayloadWithEmit.sol'; +import {PayloadWithStorage} from './mocks/PayloadWithStorage.sol'; contract ProtocolV3TestBaseTest is ProtocolV3TestBase { function setUp() public { @@ -180,3 +181,23 @@ contract ProtocolV3TestMantleSnapshot is ProtocolV3TestBase { ); } } + +contract ProtocolV3TestStorageValidation is ProtocolV3TestBase { + function test_noStorageSlots_passes() public { + // PayloadWithEmit has no state variables — should pass silently. + _validateNoPayloadStorageSlots(address(new PayloadWithEmit())); + } + + function test_withStorageSlots_reverts() public { + address payload = address(new PayloadWithStorage()); + // PayloadWithStorage declares `uint256 internal _randomStorageVariable` — must be rejected. + vm.expectRevert(); + _validateNoPayloadStorageSlots(payload); + } + + function test_unknownArtifact_logsWarning() public { + // makeAddr produces an address with no deployed code; getArtifactPathByDeployedCode + // cannot resolve it, so the function logs a warning and returns without reverting. + _validateNoPayloadStorageSlots(makeAddr('unknownPayload')); + } +} diff --git a/tests/mocks/PayloadWithStorage.sol b/tests/mocks/PayloadWithStorage.sol new file mode 100644 index 000000000..1f7a720d4 --- /dev/null +++ b/tests/mocks/PayloadWithStorage.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.10; + +import {IProposalGenericExecutor} from '../../src/interfaces/IProposalGenericExecutor.sol'; + +/** + * @dev Mock payload that incorrectly declares a state variable. + * Used to test that _validateNoPayloadStorageSlots detects storage variables. + */ +contract PayloadWithStorage is IProposalGenericExecutor { + uint256 internal _randomStorageVariable; + + function execute() external { + // do nothing just relax + } +} From ee3bf03ae575f33e306bffdcd1aaaa0ceda22f35 Mon Sep 17 00:00:00 2001 From: Harsh Pandey Date: Thu, 26 Feb 2026 23:18:22 +0530 Subject: [PATCH 2/5] fix: force validation if artifacts does not exist --- src/ProtocolV3TestBase.sol | 18 ++++-------------- tests/ProtocolV3TestBase.t.sol | 3 ++- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/ProtocolV3TestBase.sol b/src/ProtocolV3TestBase.sol index d3151f53a..d433744bc 100644 --- a/src/ProtocolV3TestBase.sol +++ b/src/ProtocolV3TestBase.sol @@ -737,25 +737,15 @@ contract ProtocolV3TestBase is RawProtocolV3TestBase, SeatbeltUtils, CommonTestB * @dev Validates that a payload contract declares no state variables by inspecting the * compiler-generated storage layout from the build artifact. * - * If the artifact cannot be resolved (e.g. the contract was not compiled locally), a - * warning is logged and the check is skipped rather than failing the test. + * If the artifact cannot be resolved (e.g. the contract was not compiled locally), + * you can skip this test manually by overriding this virtual method in your test * * Requires foundry.toml to have: * extra_output = ["storageLayout"] * fs_permissions includes { access = "read", path = "./out" } */ - function _validateNoPayloadStorageSlots(address payload) internal view { - string memory artifactPath; - try vm.getArtifactPathByDeployedCode(payload.code) returns (string memory path) { - artifactPath = path; - } catch { - console.log( - 'WARNING: _validateNoPayloadStorageSlots: could not resolve artifact for payload %s, skipping storage slot check', - payload - ); - return; - } - + function _validateNoPayloadStorageSlots(address payload) internal view virtual { + string memory artifactPath = vm.getArtifactPathByDeployedCode(payload.code); string memory artifact = vm.readFile(artifactPath); // vm.parseJson ABI-encodes the JSON value at the given key. diff --git a/tests/ProtocolV3TestBase.t.sol b/tests/ProtocolV3TestBase.t.sol index 6c64b285f..b879c4edb 100644 --- a/tests/ProtocolV3TestBase.t.sol +++ b/tests/ProtocolV3TestBase.t.sol @@ -197,7 +197,8 @@ contract ProtocolV3TestStorageValidation is ProtocolV3TestBase { function test_unknownArtifact_logsWarning() public { // makeAddr produces an address with no deployed code; getArtifactPathByDeployedCode - // cannot resolve it, so the function logs a warning and returns without reverting. + // cannot resolve it, so the function vm.getArtifactPathByDeployedCode reverts + vm.expectRevert(); _validateNoPayloadStorageSlots(makeAddr('unknownPayload')); } } From 79a2e77d9007c852b34e40e3f842024d37609c64 Mon Sep 17 00:00:00 2001 From: Harsh Pandey Date: Thu, 26 Feb 2026 23:28:01 +0530 Subject: [PATCH 3/5] chore: add for v2 as well, just in case --- src/ProtocolV2TestBase.sol | 40 ++++++++++++++++++++++++++++++++++ src/ProtocolV3TestBase.sol | 4 ++-- tests/ProtocolV2TestBase.t.sol | 22 +++++++++++++++++++ 3 files changed, 64 insertions(+), 2 deletions(-) diff --git a/src/ProtocolV2TestBase.sol b/src/ProtocolV2TestBase.sol index bde085b7a..ccda6feca 100644 --- a/src/ProtocolV2TestBase.sol +++ b/src/ProtocolV2TestBase.sol @@ -7,6 +7,7 @@ import {ReserveConfiguration} from 'aave-v3-origin/contracts/protocol/libraries/ import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/IERC20.sol'; import {IERC20Metadata} from 'openzeppelin-contracts/contracts/token/ERC20/extensions/IERC20Metadata.sol'; import {SafeERC20} from 'openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol'; +import {Strings} from 'openzeppelin-contracts/contracts/utils/Strings.sol'; import {AaveV2EthereumAMM} from 'aave-address-book/AaveV2EthereumAMM.sol'; import {AaveV2EthereumAssets} from 'aave-address-book/AaveV2Ethereum.sol'; import {DiffUtils} from './DiffUtils.sol'; @@ -85,6 +86,10 @@ contract ProtocolV2TestBase is CommonTestBase, SeatbeltUtils, DiffUtils { string memory afterString = string(abi.encodePacked(reportName, '_after')); ReserveConfig[] memory configAfter = createConfigurationSnapshot(afterString, pool); + + // as executor does delegateCall to the payload, the payload should have no storage variable + _validateNoPayloadStorageSlots(payload); + vm.writeJson( vm.serializeString('root', 'raw', rawDiff), // output string(abi.encodePacked('./reports/', afterString, '.json')) @@ -993,6 +998,41 @@ contract ProtocolV2TestBase is CommonTestBase, SeatbeltUtils, DiffUtils { ); } + /** + * @dev Validates that a payload contract declares no state variables by inspecting the + * compiler-generated storage layout from the build artifact. + * + * If the artifact cannot be resolved (e.g. the contract was not compiled locally), + * you can skip this test manually by overriding this virtual method in your test + * + * Requires foundry.toml to have: + * extra_output = ["storageLayout"] + * fs_permissions includes { access = "read", path = "./out" } + */ + function _validateNoPayloadStorageSlots(address payload) internal view virtual { + string memory artifactPath = vm.getArtifactPathByDeployedCode(payload.code); + string memory artifact = vm.readFile(artifactPath); + + // vm.parseJson ABI-encodes the JSON value at the given key. + // A dynamic array is encoded as [uint256 offset][uint256 length][elements...], so + // decoding the first two words as (uint256, uint256) yields (offset, arrayLength). + bytes memory storageEncoded = vm.parseJson(artifact, '.storageLayout.storage'); + (, uint256 storageLength) = abi.decode(storageEncoded, (uint256, uint256)); + + require( + storageLength == 0, + string( + abi.encodePacked( + 'PAYLOAD_MUST_NOT_HAVE_STORAGE_VARIABLES: ', + artifactPath, + ' declares ', + Strings.toString(storageLength), + ' storage slot(s)' + ) + ) + ); + } + function _isInUint256Array( uint256[] memory haystack, uint256 needle diff --git a/src/ProtocolV3TestBase.sol b/src/ProtocolV3TestBase.sol index d433744bc..430399690 100644 --- a/src/ProtocolV3TestBase.sol +++ b/src/ProtocolV3TestBase.sol @@ -94,11 +94,11 @@ contract ProtocolV3TestBase is RawProtocolV3TestBase, SeatbeltUtils, CommonTestB uint256 gasUsed = startGas - gasleft(); assertLt(gasUsed, (block.gaslimit * 95) / 100, 'BLOCK_GAS_LIMIT_EXCEEDED'); // 5% is kept as a buffer + ReserveConfig[] memory configAfter = createConfigurationSnapshot(afterString, pool); + // as executor does delegateCall to the payload, the payload should have no storage variable _validateNoPayloadStorageSlots(payload); - ReserveConfig[] memory configAfter = createConfigurationSnapshot(afterString, pool); - { string memory rawDiff = vm.getStateDiffJson(); vm.writeJson(rawDiff, string(abi.encodePacked('./reports/', afterString, '.json')), '$.raw'); diff --git a/tests/ProtocolV2TestBase.t.sol b/tests/ProtocolV2TestBase.t.sol index 77df227a3..283c879e9 100644 --- a/tests/ProtocolV2TestBase.t.sol +++ b/tests/ProtocolV2TestBase.t.sol @@ -7,6 +7,7 @@ import {AaveV2Ethereum, AaveV2EthereumAssets} from 'aave-address-book/AaveV2Ethe import {AaveV2EthereumAMM} from 'aave-address-book/AaveV2EthereumAMM.sol'; import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/IERC20.sol'; import {PayloadWithEmit} from './mocks/PayloadWithEmit.sol'; +import {PayloadWithStorage} from './mocks/PayloadWithStorage.sol'; contract ProtocolV2TestBaseTest is ProtocolV2TestBase { function setUp() public { @@ -40,3 +41,24 @@ contract ProtocolV2TestE2ETestAsset is ProtocolV2TestBase { defaultTest('AMMTEST', AaveV2EthereumAMM.POOL, address(new PayloadWithEmit()), false, false); } } + +contract ProtocolV2TestStorageValidation is ProtocolV2TestBase { + function test_noStorageSlots_passes() public { + // PayloadWithEmit has no state variables — should pass silently. + _validateNoPayloadStorageSlots(address(new PayloadWithEmit())); + } + + function test_withStorageSlots_reverts() public { + address payload = address(new PayloadWithStorage()); + // PayloadWithStorage declares `uint256 internal _randomStorageVariable` — must be rejected. + vm.expectRevert(); + _validateNoPayloadStorageSlots(payload); + } + + function test_unknownArtifact_logsWarning() public { + // makeAddr produces an address with no deployed code; getArtifactPathByDeployedCode + // cannot resolve it, so the function vm.getArtifactPathByDeployedCode reverts + vm.expectRevert(); + _validateNoPayloadStorageSlots(makeAddr('unknownPayload')); + } +} \ No newline at end of file From fb03b9e931f37f7e1ff9db6cfa19422bde6a2935 Mon Sep 17 00:00:00 2001 From: Harsh Pandey Date: Thu, 26 Feb 2026 23:29:43 +0530 Subject: [PATCH 4/5] chore: fix lint --- src/ProtocolV2TestBase.sol | 2 +- src/ProtocolV3TestBase.sol | 2 +- tests/ProtocolV2TestBase.t.sol | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ProtocolV2TestBase.sol b/src/ProtocolV2TestBase.sol index ccda6feca..9e2a2ff6c 100644 --- a/src/ProtocolV2TestBase.sol +++ b/src/ProtocolV2TestBase.sol @@ -1002,7 +1002,7 @@ contract ProtocolV2TestBase is CommonTestBase, SeatbeltUtils, DiffUtils { * @dev Validates that a payload contract declares no state variables by inspecting the * compiler-generated storage layout from the build artifact. * - * If the artifact cannot be resolved (e.g. the contract was not compiled locally), + * If the artifact cannot be resolved (e.g. the contract was not compiled locally), * you can skip this test manually by overriding this virtual method in your test * * Requires foundry.toml to have: diff --git a/src/ProtocolV3TestBase.sol b/src/ProtocolV3TestBase.sol index 430399690..9c9166082 100644 --- a/src/ProtocolV3TestBase.sol +++ b/src/ProtocolV3TestBase.sol @@ -737,7 +737,7 @@ contract ProtocolV3TestBase is RawProtocolV3TestBase, SeatbeltUtils, CommonTestB * @dev Validates that a payload contract declares no state variables by inspecting the * compiler-generated storage layout from the build artifact. * - * If the artifact cannot be resolved (e.g. the contract was not compiled locally), + * If the artifact cannot be resolved (e.g. the contract was not compiled locally), * you can skip this test manually by overriding this virtual method in your test * * Requires foundry.toml to have: diff --git a/tests/ProtocolV2TestBase.t.sol b/tests/ProtocolV2TestBase.t.sol index 283c879e9..73bbe74c3 100644 --- a/tests/ProtocolV2TestBase.t.sol +++ b/tests/ProtocolV2TestBase.t.sol @@ -61,4 +61,4 @@ contract ProtocolV2TestStorageValidation is ProtocolV2TestBase { vm.expectRevert(); _validateNoPayloadStorageSlots(makeAddr('unknownPayload')); } -} \ No newline at end of file +} From 6e9b2b7d7f4be18986cad2f797c161eec923ae09 Mon Sep 17 00:00:00 2001 From: Harsh Pandey Date: Thu, 26 Feb 2026 23:36:12 +0530 Subject: [PATCH 5/5] fix: test --- tests/ProtocolV3TestBase.t.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ProtocolV3TestBase.t.sol b/tests/ProtocolV3TestBase.t.sol index b879c4edb..49ebb88c0 100644 --- a/tests/ProtocolV3TestBase.t.sol +++ b/tests/ProtocolV3TestBase.t.sol @@ -180,6 +180,9 @@ contract ProtocolV3TestMantleSnapshot is ProtocolV3TestBase { false ); } + + // overriding the storage slot check as payload artifacts does not exists + function _validateNoPayloadStorageSlots(address payload) internal view override {} } contract ProtocolV3TestStorageValidation is ProtocolV3TestBase {