test: regression tests for issues in v30 audit

l1-contracts/contracts/interop/InteropCenter.sol

@@ -17,7 +17,7 @@ import {BUNDLE_IDENTIFIER, BalanceChange, BundleAttributes, CallAttributes, INTE
 import {MsgValueMismatch, NotL1, NotL2ToL2, Unauthorized} from "../common/L1ContractErrors.sol";
 import {NotInGatewayMode} from "../core/bridgehub/L1BridgehubErrors.sol";
 
-import {AttributeAlreadySet, AttributeViolatesRestriction, IndirectCallValueMismatch, InteroperableAddressChainReferenceNotEmpty, InteroperableAddressNotEmpty} from "./InteropErrors.sol";
+import {AttributeAlreadySet, AttributeViolatesRestriction, DestinationChainNotRegistered, IndirectCallValueMismatch, InteroperableAddressChainReferenceNotEmpty, InteroperableAddressNotEmpty} from "./InteropErrors.sol";
 
 import {IERC7786GatewaySource} from "./IERC7786GatewaySource.sol";
 import {IERC7786Attributes} from "./IERC7786Attributes.sol";
@@ -262,6 +262,7 @@ contract InteropCenter is
         uint256 _totalIndirectCallsValue
     ) internal {
         bytes32 destinationChainBaseTokenAssetId = L2_BRIDGEHUB.baseTokenAssetId(_destinationChainId);
+        require(destinationChainBaseTokenAssetId != bytes32(0), DestinationChainNotRegistered(_destinationChainId));
         // We burn the value that is passed along the bundle here, on source chain.
         bytes32 thisChainBaseTokenAssetId = L2_BRIDGEHUB.baseTokenAssetId(block.chainid);
         if (destinationChainBaseTokenAssetId == thisChainBaseTokenAssetId) {

l1-contracts/contracts/interop/InteropErrors.sol

@@ -15,6 +15,8 @@ error CallAlreadyExecuted(bytes32 bundleHash, uint256 callIndex);
 error CallNotExecutable(bytes32 bundleHash, uint256 callIndex);
 // 0xf729f26d
 error CanNotUnbundle(bytes32 bundleHash);
+// 0x2d159f39
+error DestinationChainNotRegistered(uint256 destinationChainId);
 // 0xe845be4c
 error ExecutingNotAllowed(bytes32 bundleHash, bytes callerAddress, bytes executionAddress);
 // 0x62d214aa

l1-contracts/selectors

@@ -6708,6 +6708,8 @@ InteropCenter
 |----------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
 | Error    | AttributeViolatesRestriction(bytes4,uint256)                                                                                    | 0xbcb41ec7                                                         |
 |----------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
+| Error    | DestinationChainNotRegistered(uint256)                                                                                          | 0x2d159f39                                                         |
+|----------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
 | Error    | IndirectCallValueMismatch(uint256,uint256)                                                                                      | 0x62d214aa                                                         |
 |----------+---------------------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
 | Error    | InteroperableAddressChainReferenceNotEmpty(bytes)                                                                               | 0xfe8b1b16                                                         |

l1-contracts/test/foundry/l1/integration/AssetTrackerTest.t.sol

@@ -37,6 +37,7 @@ import {INativeTokenVaultBase} from "contracts/bridge/ntv/INativeTokenVaultBase.
 import {IERC20} from "@openzeppelin/contracts-v4/token/ERC20/IERC20.sol";
 
 import {IAssetTrackerBase} from "contracts/bridge/asset-tracker/IAssetTrackerBase.sol";
+import {InvalidChainId} from "contracts/common/L1ContractErrors.sol";
 
 contract AssetTrackerTests is L1ContractDeployer, ZKChainDeployer, TokenDeployer, L2TxMocker {
     using stdStorage for StdStorage;
@@ -593,6 +594,123 @@ contract AssetTrackerTests is L1ContractDeployer, ZKChainDeployer, TokenDeployer
         IAssetTrackerBase(address(assetTracker)).tokenMigratedThisChain(bytes32(0));
     }
 
+    function test_regression_migrateTokenBalanceFromNTVV31_revertsForUnknownAsset() public {
+        // Create a predictable future assetId that is NOT registered in NTV
+        bytes32 unknownAssetId = keccak256("unknown-asset-never-registered");
+        uint256 testChainId = 999; // Some chain that's not the origin (since _chainId != originChainId is required)
+
+        // Mock the NTV to return 0 for originChainId (simulating unknown asset)
+        vm.mockCall(
+            address(ecosystemAddresses.bridges.proxies.l1NativeTokenVault),
+            abi.encodeWithSelector(INativeTokenVaultBase.originChainId.selector, unknownAssetId),
+            abi.encode(0) // Unknown asset returns 0
+        );
+
+        // Verify initial state: chainBalance[0][unknownAssetId] should be 0
+        bytes32 chainBalanceSlot0 = getChainBalanceLocation(unknownAssetId, 0);
+        uint256 initialChainBalance0 = uint256(vm.load(address(assetTracker), chainBalanceSlot0));
+        assertEq(initialChainBalance0, 0, "Initial chainBalance[0] should be 0");
+
+        // Attempt to migrate the unknown asset - should revert with InvalidChainId
+        // Before the fix, this would succeed and poison state
+        vm.expectRevert(InvalidChainId.selector);
+        assetTracker.migrateTokenBalanceFromNTVV31(testChainId, unknownAssetId);
+
+        // Verify state was NOT poisoned (chainBalance[0][unknownAssetId] should still be 0)
+        uint256 finalChainBalance0 = uint256(vm.load(address(assetTracker), chainBalanceSlot0));
+        assertEq(finalChainBalance0, 0, "chainBalance[0] should not have been set to MAX_TOKEN_BALANCE");
+    }
+
+    function test_regression_migrateTokenBalanceFromNTVV31_preventsStatePoisoning() public {
+        // Attacker picks a predictable future assetId (unknown to NTV: originChainId(assetId) == 0)
+        bytes32 futureAssetId = keccak256(abi.encodePacked("future-token-", block.timestamp));
+        uint256 attackerChainId = 12345;
+
+        // Mock NTV to return 0 for this "future" asset
+        vm.mockCall(
+            address(ecosystemAddresses.bridges.proxies.l1NativeTokenVault),
+            abi.encodeWithSelector(INativeTokenVaultBase.originChainId.selector, futureAssetId),
+            abi.encode(0)
+        );
+
+        // Before the fix, this attack would succeed:
+        // 1. Call migrateTokenBalanceFromNTVV31(attackerChainId, futureAssetId)
+        // 2. originChainId = 0 (for unknown asset)
+        // 3. Since attackerChainId != 0, the require(_chainId != originChainId) passes
+        // 4. migrateTokenBalanceToAssetTracker returns 0 (no balance)
+        // 5. _assignMaxChainBalanceIfNeeded(0, futureAssetId) sets chainBalance[0][futureAssetId] = MAX
+        // 6. State is now poisoned - maxChainBalanceAssigned[futureAssetId] = true
+
+        // After the fix, this should revert immediately
+        vm.expectRevert(InvalidChainId.selector);
+        assetTracker.migrateTokenBalanceFromNTVV31(attackerChainId, futureAssetId);
+    }
+
+    /// @notice Test that registered assets can still be migrated correctly
+    /// @dev Ensures the fix doesn't break legitimate migration operations
+    function test_regression_migrateTokenBalanceFromNTVV31_worksForRegisteredAsset() public {
+        // Use an already registered asset
+        uint256 testChainId = eraZKChainId;
+        uint256 migratedBalance = 5000;
+
+        // Mock origin chain to be different from testChainId and non-zero
+        uint256 registeredOriginChain = originalChainId;
+        vm.mockCall(
+            address(ecosystemAddresses.bridges.proxies.l1NativeTokenVault),
+            abi.encodeWithSelector(INativeTokenVaultBase.originChainId.selector, assetId),
+            abi.encode(registeredOriginChain)
+        );
+
+        // Mock the migration to return a balance
+        vm.mockCall(
+            address(ecosystemAddresses.bridges.proxies.l1NativeTokenVault),
+            abi.encodeWithSelector(
+                IL1NativeTokenVault.migrateTokenBalanceToAssetTracker.selector,
+                testChainId,
+                assetId
+            ),
+            abi.encode(migratedBalance)
+        );
+
+        // Set initial origin chain balance
+        vm.store(
+            address(assetTracker),
+            getChainBalanceLocation(assetId, registeredOriginChain),
+            bytes32(type(uint256).max)
+        );
+
+        // This should succeed for a registered asset (originChainId != 0)
+        assetTracker.migrateTokenBalanceFromNTVV31(testChainId, assetId);
+
+        // Verify balance was migrated correctly
+        uint256 testChainBalance = uint256(
+            vm.load(address(assetTracker), getChainBalanceLocation(assetId, testChainId))
+        );
+        assertEq(testChainBalance, migratedBalance, "Test chain should have migrated balance");
+    }
+
+    /// @notice Fuzz test for unknown assetId rejection
+    /// @dev Ensures any random assetId that returns originChainId=0 is rejected
+    function testFuzz_regression_migrateTokenBalanceFromNTVV31_revertsForAnyUnknownAsset(
+        bytes32 randomAssetId,
+        uint256 randomChainId
+    ) public {
+        // Ensure chain ID is not 0 (would fail the != originChainId check anyway)
+        vm.assume(randomChainId != 0);
+        vm.assume(randomChainId != block.chainid); // Skip L1 chain case
+
+        // Mock NTV to return 0 for this random asset (simulating unknown)
+        vm.mockCall(
+            address(ecosystemAddresses.bridges.proxies.l1NativeTokenVault),
+            abi.encodeWithSelector(INativeTokenVaultBase.originChainId.selector, randomAssetId),
+            abi.encode(0)
+        );
+
+        // Should always revert for unknown assets
+        vm.expectRevert(InvalidChainId.selector);
+        assetTracker.migrateTokenBalanceFromNTVV31(randomChainId, randomAssetId);
+    }
+
     // add this to be excluded from coverage report
     function test() internal override {}
 }

l1-contracts/test/foundry/l1/integration/l2-tests-abstract/L2AssetRouterAttributesEncodingRegressionTestAbstract.t.sol

@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+// solhint-disable gas-custom-errors
+
+import {StdStorage, Test, stdStorage} from "forge-std/Test.sol";
+import "forge-std/console.sol";
+
+import {IERC7786Attributes} from "contracts/interop/IERC7786Attributes.sol";
+import {InteropCallStarter, CallAttributes} from "contracts/common/Messaging.sol";
+import {InteroperableAddress} from "contracts/vendor/draft-InteroperableAddress.sol";
+import {AttributesDecoder} from "contracts/interop/AttributesDecoder.sol";
+import {InteropCenter} from "contracts/interop/InteropCenter.sol";
+import {IInteropCenter} from "contracts/interop/IInteropCenter.sol";
+
+import {L2_INTEROP_CENTER_ADDR} from "contracts/common/l2-helpers/L2ContractAddresses.sol";
+
+import {SharedL2ContractDeployer} from "./_SharedL2ContractDeployer.sol";
+
+/// @title L2AssetRouterAttributesEncodingRegressionTestAbstract
+/// @notice Regression tests for the callAttributes encoding fix in L2AssetRouter.initiateIndirectCall
+abstract contract L2AssetRouterAttributesEncodingRegressionTestAbstract is Test, SharedL2ContractDeployer {
+    uint256 destinationChainId = 271;
+
+    function setUp() public virtual override {
+        super.setUp();
+    }
+
+    /// @notice Test that abi.encodeCall produces the correct format for parseAttributes
+    /// @dev This is a unit test verifying the encoding format difference
+    function test_regression_abiEncodeCallVsAbiEncode() public pure {
+        uint256 testValue = 12345;
+        bytes4 selector = IERC7786Attributes.interopCallValue.selector;
+
+        // CORRECT encoding using abi.encodeCall
+        // Format: [4 bytes selector][32 bytes value] = 36 bytes
+        bytes memory correctEncoding = abi.encodeCall(IERC7786Attributes.interopCallValue, testValue);
+
+        // BUGGY encoding using abi.encode (the bug that was fixed)
+        // Format: [4 bytes selector][28 bytes zero padding][32 bytes value] = 64 bytes
+        // Note: abi.encode pads bytes4 to 32 bytes with right-padding (selector is left-aligned)
+        bytes memory buggyEncoding = abi.encode(selector, testValue);
+
+        // Verify the lengths are different
+        assertEq(correctEncoding.length, 36, "Correct encoding should be 36 bytes (4 selector + 32 value)");
+        assertEq(buggyEncoding.length, 64, "Buggy encoding should be 64 bytes (32 padded selector + 32 value)");
+
+        // Verify the selector is at the start of both (bytes4 is left-aligned in abi.encode)
+        bytes4 correctSelector = bytes4(correctEncoding);
+        bytes4 buggySelector = bytes4(buggyEncoding);
+        assertEq(correctSelector, selector, "Correct encoding should have selector at start");
+        assertEq(buggySelector, selector, "Buggy encoding also has selector at start (left-aligned)");
+
+        // The KEY difference: where the value is located
+        // In correct encoding: value is at bytes[4:36]
+        // In buggy encoding: bytes[4:36] contains 28 zeros + first 4 bytes of value
+        // The actual value is at bytes[32:64] in buggy encoding
+
+        // When AttributesDecoder reads _data[4:], it gets different data:
+        // - Correct: [32 bytes value] -> decodes to testValue
+        // - Buggy: [28 bytes zeros][first 4 bytes of value] -> decodes to wrong value
+    }
+
+    /// @notice Test that InteropCenter.parseAttributes correctly decodes the attributes from L2AssetRouter
+    /// @dev This verifies the fix works end-to-end with parseAttributes
+    function test_regression_parseAttributesDecodesCorrectly() public view {
+        uint256 testValue = 1 ether;
+
+        // Create attributes using the CORRECT encoding (as fixed in PR #1714)
+        bytes[] memory attributes = new bytes[](1);
+        attributes[0] = abi.encodeCall(IERC7786Attributes.interopCallValue, testValue);
+
+        // Call parseAttributes to decode
+        (CallAttributes memory callAttributes, ) = InteropCenter(L2_INTEROP_CENTER_ADDR).parseAttributes(
+            attributes,
+            IInteropCenter.AttributeParsingRestrictions.OnlyInteropCallValue
+        );
+
+        // Verify the value was decoded correctly
+        assertEq(
+            callAttributes.interopCallValue,
+            testValue,
+            "parseAttributes should decode the interopCallValue correctly"
+        );
+    }
+
+    /// @notice Test that the buggy encoding would cause parseAttributes to return wrong value
+    /// @dev This demonstrates the bug that was fixed
+    function test_regression_buggyEncodingWouldReturnWrongValue() public view {
+        uint256 testValue = 1 ether;
+        bytes4 selector = IERC7786Attributes.interopCallValue.selector;
+
+        // Create attributes using the BUGGY encoding (abi.encode instead of abi.encodeCall)
+        bytes[] memory buggyAttributes = new bytes[](1);
+        buggyAttributes[0] = abi.encode(selector, testValue);
+
+        // Call parseAttributes to decode - this will return a wrong value
+        (CallAttributes memory callAttributes, ) = InteropCenter(L2_INTEROP_CENTER_ADDR).parseAttributes(
+            buggyAttributes,
+            IInteropCenter.AttributeParsingRestrictions.OnlyInteropCallValue
+        );
+
+        // The decoded value should NOT equal the original value
+        // because parseAttributes reads from _data[4:] which reads the padded selector bytes
+        assertNotEq(
+            callAttributes.interopCallValue,
+            testValue,
+            "Buggy encoding should NOT decode to the correct value"
+        );
+    }
+
+    /// @notice Fuzz test for various values
+    /// @dev Ensures the encoding works for any uint256 value
+    function testFuzz_regression_attributesEncodingVariousValues(uint256 testValue) public view {
+        // Create attributes using the correct encoding
+        bytes[] memory attributes = new bytes[](1);
+        attributes[0] = abi.encodeCall(IERC7786Attributes.interopCallValue, testValue);
+
+        // Verify parseAttributes decodes correctly
+        (CallAttributes memory callAttributes, ) = InteropCenter(L2_INTEROP_CENTER_ADDR).parseAttributes(
+            attributes,
+            IInteropCenter.AttributeParsingRestrictions.OnlyInteropCallValue
+        );
+
+        assertEq(callAttributes.interopCallValue, testValue, "parseAttributes should decode any value correctly");
+    }
+}