Add an optional parameters to the InteroperableAddress parsing functions

.changeset/tall-aliens-matter.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`InteroperableAddress`: Add an optional `failOnExtraBytes` parameter to all parsing functions. If enabled, the parsing will reject any input that contains extra bytes.

contracts/utils/draft-InteroperableAddress.sol

@@ -74,20 +74,52 @@ library InteroperableAddress {
      */
     function parseV1(
         bytes memory self
+    ) internal pure returns (bytes2 chainType, bytes memory chainReference, bytes memory addr) {
+        return parseV1(self, false);
+    }
+
+    /**
+     * @dev Parse a ERC-7930 interoperable address (version 1) into its different components. Reverts if the input is
+     * not following a version 1 of ERC-7930.
+     *
+     * NOTE: If `failOnExtraBytes` is false, trailing bytes after a valid v1 encoding are ignored. The same decoded
+     * address may therefore correspond to multiple distinct input byte strings. If `failOnExtraBytes` is true, then
+     * any trailing byte will cause this function to revert.
+     */
+    function parseV1(
+        bytes memory self,
+        bool failOnExtraBytes
     ) internal pure returns (bytes2 chainType, bytes memory chainReference, bytes memory addr) {
         bool success;
-        (success, chainType, chainReference, addr) = tryParseV1(self);
+        (success, chainType, chainReference, addr) = tryParseV1(self, failOnExtraBytes);
         require(success, InteroperableAddressParsingError(self));
     }
 
     /**
      * @dev Variant of {parseV1} that handles calldata slices to reduce memory copy costs.
+     *
+     * NOTE: Trailing bytes after a valid v1 encoding are ignored. The same decoded address may therefore correspond
+     * to multiple distinct input byte strings.
      */
     function parseV1Calldata(
         bytes calldata self
+    ) internal pure returns (bytes2 chainType, bytes calldata chainReference, bytes calldata addr) {
+        return parseV1Calldata(self, false);
+    }
+
+    /**
+     * @dev Variant of {parseV1} that handles calldata slices to reduce memory copy costs.
+     *
+     * NOTE: If `failOnExtraBytes` is false, trailing bytes after a valid v1 encoding are ignored. The same decoded
+     * address may therefore correspond to multiple distinct input byte strings. If `failOnExtraBytes` is true, then
+     * any trailing byte will cause this function to revert.
+     */
+    function parseV1Calldata(
+        bytes calldata self,
+        bool failOnExtraBytes
     ) internal pure returns (bytes2 chainType, bytes calldata chainReference, bytes calldata addr) {
         bool success;
-        (success, chainType, chainReference, addr) = tryParseV1Calldata(self);
+        (success, chainType, chainReference, addr) = tryParseV1Calldata(self, failOnExtraBytes);
         require(success, InteroperableAddressParsingError(self));
     }
 
@@ -97,6 +129,19 @@ library InteroperableAddress {
      */
     function tryParseV1(
         bytes memory self
+    ) internal pure returns (bool success, bytes2 chainType, bytes memory chainReference, bytes memory addr) {
+        return tryParseV1(self, false);
+    }
+
+    /**
+     * @dev Variant of {parseV1} that does not revert on invalid input. Instead, it returns `false` as the first
+     * return value to indicate parsing failure when the input does not follow version 1 of ERC-7930.
+     *
+     * The extra `failOnExtraBytes` parameter can be used to reject input that have trailing bytes.
+     */
+    function tryParseV1(
+        bytes memory self,
+        bool failOnExtraBytes
     ) internal pure returns (bool success, bytes2 chainType, bytes memory chainReference, bytes memory addr) {
         unchecked {
             if (self.length < 0x06) return (false, 0x0000, _emptyBytesMemory(), _emptyBytesMemory());
@@ -110,9 +155,10 @@ library InteroperableAddress {
             chainReference = self.slice(0x05, 0x05 + chainReferenceLength);
 
             uint256 addrLength = uint8(self[0x05 + chainReferenceLength]);
-            if (self.length < 0x06 + chainReferenceLength + addrLength)
+            uint256 expectedLength = 0x06 + chainReferenceLength + addrLength;
+            if ((self.length < expectedLength) || (failOnExtraBytes && self.length > expectedLength))
                 return (false, 0x0000, _emptyBytesMemory(), _emptyBytesMemory());
-            addr = self.slice(0x06 + chainReferenceLength, 0x06 + chainReferenceLength + addrLength);
+            addr = self.slice(0x06 + chainReferenceLength, expectedLength);
 
             // At least one of chainReference or addr must be non-empty
             success = (chainReferenceLength > 0) || (addrLength > 0);
@@ -125,6 +171,18 @@ library InteroperableAddress {
      */
     function tryParseV1Calldata(
         bytes calldata self
+    ) internal pure returns (bool success, bytes2 chainType, bytes calldata chainReference, bytes calldata addr) {
+        return tryParseV1Calldata(self, false);
+    }
+
+    /**
+     * @dev Variant of {tryParseV1} that handles calldata slices to reduce memory copy costs.
+     *
+     * The extra `failOnExtraBytes` parameter can be used to reject input that have trailing bytes.
+     */
+    function tryParseV1Calldata(
+        bytes calldata self,
+        bool failOnExtraBytes
     ) internal pure returns (bool success, bytes2 chainType, bytes calldata chainReference, bytes calldata addr) {
         unchecked {
             if (self.length < 0x06) return (false, 0x0000, Calldata.emptyBytes(), Calldata.emptyBytes());
@@ -138,9 +196,10 @@ library InteroperableAddress {
             chainReference = self[0x05:0x05 + chainReferenceLength];
 
             uint256 addrLength = uint8(self[0x05 + chainReferenceLength]);
-            if (self.length < 0x06 + chainReferenceLength + addrLength)
+            uint256 expectedLength = 0x06 + chainReferenceLength + addrLength;
+            if ((self.length < expectedLength) || (failOnExtraBytes && self.length > expectedLength))
                 return (false, 0x0000, Calldata.emptyBytes(), Calldata.emptyBytes());
-            addr = self[0x06 + chainReferenceLength:0x06 + chainReferenceLength + addrLength];
+            addr = self[0x06 + chainReferenceLength:expectedLength];
 
             // At least one of chainReference or addr must be non-empty
             success = (chainReferenceLength > 0) || (addrLength > 0);
@@ -161,17 +220,37 @@ library InteroperableAddress {
      * * The underlying chainType must be "eip-155"
      */
     function parseEvmV1(bytes memory self) internal pure returns (uint256 chainId, address addr) {
+        return parseEvmV1(self, false);
+    }
+
+    /**
+     * @dev Variant of {parseEvmV1} with the `failOnExtraBytes` parameter.
+     */
+    function parseEvmV1(
+        bytes memory self,
+        bool failOnExtraBytes
+    ) internal pure returns (uint256 chainId, address addr) {
         bool success;
-        (success, chainId, addr) = tryParseEvmV1(self);
+        (success, chainId, addr) = tryParseEvmV1(self, failOnExtraBytes);
         require(success, InteroperableAddressParsingError(self));
     }
 
     /**
      * @dev Variant of {parseEvmV1} that handles calldata slices to reduce memory copy costs.
      */
     function parseEvmV1Calldata(bytes calldata self) internal pure returns (uint256 chainId, address addr) {
+        return parseEvmV1Calldata(self, false);
+    }
+
+    /**
+     * @dev Variant of {parseEvmV1} that handles calldata slices to reduce memory copy costs and that supports the `failOnExtraBytes` parameter.
+     */
+    function parseEvmV1Calldata(
+        bytes calldata self,
+        bool failOnExtraBytes
+    ) internal pure returns (uint256 chainId, address addr) {
         bool success;
-        (success, chainId, addr) = tryParseEvmV1Calldata(self);
+        (success, chainId, addr) = tryParseEvmV1Calldata(self, failOnExtraBytes);
         require(success, InteroperableAddressParsingError(self));
     }
 
@@ -180,7 +259,22 @@ library InteroperableAddress {
      * return value to indicate parsing failure when the input does not follow version 1 of ERC-7930.
      */
     function tryParseEvmV1(bytes memory self) internal pure returns (bool success, uint256 chainId, address addr) {
-        (bool success_, bytes2 chainType_, bytes memory chainReference_, bytes memory addr_) = tryParseV1(self);
+        return tryParseEvmV1(self, false);
+    }
+
+    /**
+     * @dev Variant of {parseEvmV1} that does not revert on invalid input. Instead, it returns `false` as the first
+     * return value to indicate parsing failure when the input does not follow version 1 of ERC-7930. Supports the
+     * `failOnExtraBytes` parameter.
+     */
+    function tryParseEvmV1(
+        bytes memory self,
+        bool failOnExtraBytes
+    ) internal pure returns (bool success, uint256 chainId, address addr) {
+        (bool success_, bytes2 chainType_, bytes memory chainReference_, bytes memory addr_) = tryParseV1(
+            self,
+            failOnExtraBytes
+        );
         return
             (success_ &&
                 chainType_ == 0x0000 &&
@@ -199,9 +293,21 @@ library InteroperableAddress {
      */
     function tryParseEvmV1Calldata(
         bytes calldata self
+    ) internal pure returns (bool success, uint256 chainId, address addr) {
+        return tryParseEvmV1Calldata(self, false);
+    }
+
+    /**
+     * @dev Variant of {tryParseEvmV1} that handles calldata slices to reduce memory copy costs and supports the
+     * `failOnExtraBytes` parameter.
+     */
+    function tryParseEvmV1Calldata(
+        bytes calldata self,
+        bool failOnExtraBytes
     ) internal pure returns (bool success, uint256 chainId, address addr) {
         (bool success_, bytes2 chainType_, bytes calldata chainReference_, bytes calldata addr_) = tryParseV1Calldata(
-            self
+            self,
+            failOnExtraBytes
         );
         return
             (success_ &&

test/utils/draft-InteroperableAddress.test.js

@@ -60,18 +60,60 @@ describe('ERC7390', function () {
     ]) {
       const { chainType, reference, address } = nameCoder.decode(name, true);
       const binary = addressCoder.encode({ chainType, reference, address });
+      const binaryExtra = ethers.concat([binary, '0x00']);
 
-      it(title, async function () {
-        const expected = [
-          chainTypeCoder.decode(chainType),
-          CAIP350[chainType].reference.decode(reference),
-          CAIP350[chainType].address.decode(address),
-        ].map(ethers.hexlify);
+      const expected = [
+        chainTypeCoder.decode(chainType),
+        CAIP350[chainType].reference.decode(reference),
+        CAIP350[chainType].address.decode(address),
+      ].map(ethers.hexlify);
 
+      it(title, async function () {
         await expect(this.mock.$parseV1(binary)).to.eventually.deep.equal(expected);
         await expect(this.mock.$parseV1Calldata(binary)).to.eventually.deep.equal(expected);
         await expect(this.mock.$tryParseV1(binary)).to.eventually.deep.equal([true, ...expected]);
         await expect(this.mock.$tryParseV1Calldata(binary)).to.eventually.deep.equal([true, ...expected]);
+
+        // default behavior: ignore trailing bytes
+        await expect(this.mock.$parseV1(binaryExtra)).to.eventually.deep.equal(expected);
+        await expect(this.mock.$parseV1Calldata(binaryExtra)).to.eventually.deep.equal(expected);
+        await expect(this.mock.$tryParseV1(binaryExtra)).to.eventually.deep.equal([true, ...expected]);
+        await expect(this.mock.$tryParseV1Calldata(binaryExtra)).to.eventually.deep.equal([true, ...expected]);
+
+        // explicitly ignore trailing bytes
+        await expect(this.mock.$parseV1(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal(expected);
+        await expect(this.mock.$parseV1Calldata(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal(
+          expected,
+        );
+        await expect(this.mock.$tryParseV1(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal([
+          true,
+          ...expected,
+        ]);
+        await expect(this.mock.$tryParseV1Calldata(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal([
+          true,
+          ...expected,
+        ]);
+
+        // reject trailing bytes
+        await expect(this.mock.$parseV1(binaryExtra, ethers.Typed.bool(true)))
+          .to.be.revertedWithCustomError(this.mock, 'InteroperableAddressParsingError')
+          .withArgs(binaryExtra);
+        await expect(this.mock.$parseV1Calldata(binaryExtra, ethers.Typed.bool(true)))
+          .to.be.revertedWithCustomError(this.mock, 'InteroperableAddressParsingError')
+          .withArgs(binaryExtra);
+        await expect(this.mock.$tryParseV1(binaryExtra, ethers.Typed.bool(true))).to.eventually.deep.equal([
+          false,
+          '0x0000',
+          '0x',
+          '0x',
+        ]);
+        await expect(this.mock.$tryParseV1Calldata(binaryExtra, ethers.Typed.bool(true))).to.eventually.deep.equal([
+          false,
+          '0x0000',
+          '0x',
+          '0x',
+        ]);
+
         await expect(this.mock.$formatV1(...expected)).to.eventually.equal(binary);
 
         if (chainType == 'eip155') {
@@ -94,6 +136,60 @@ describe('ERC7390', function () {
             address ?? ethers.ZeroAddress,
           ]);
 
+          // default behavior: ignore trailing bytes
+          await expect(this.mock.$parseEvmV1(binaryExtra)).to.eventually.deep.equal([
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+          await expect(this.mock.$parseEvmV1Calldata(binaryExtra)).to.eventually.deep.equal([
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+          await expect(this.mock.$tryParseEvmV1(binaryExtra)).to.eventually.deep.equal([
+            true,
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+          await expect(this.mock.$tryParseEvmV1Calldata(binaryExtra)).to.eventually.deep.equal([
+            true,
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+
+          // explicitly ignore trailing bytes
+          await expect(this.mock.$parseEvmV1(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal([
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+          await expect(this.mock.$parseEvmV1Calldata(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal([
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+          await expect(this.mock.$tryParseEvmV1(binaryExtra, ethers.Typed.bool(false))).to.eventually.deep.equal([
+            true,
+            reference ?? 0n,
+            address ?? ethers.ZeroAddress,
+          ]);
+          await expect(
+            this.mock.$tryParseEvmV1Calldata(binaryExtra, ethers.Typed.bool(false)),
+          ).to.eventually.deep.equal([true, reference ?? 0n, address ?? ethers.ZeroAddress]);
+
+          // reject trailing bytes
+          await expect(this.mock.$parseEvmV1(binaryExtra, ethers.Typed.bool(true)))
+            .to.be.revertedWithCustomError(this.mock, 'InteroperableAddressParsingError')
+            .withArgs(binaryExtra);
+          await expect(this.mock.$parseEvmV1Calldata(binaryExtra, ethers.Typed.bool(true)))
+            .to.be.revertedWithCustomError(this.mock, 'InteroperableAddressParsingError')
+            .withArgs(binaryExtra);
+          await expect(this.mock.$tryParseEvmV1(binaryExtra, ethers.Typed.bool(true))).to.eventually.deep.equal([
+            false,
+            0n,
+            ethers.ZeroAddress,
+          ]);
+          await expect(this.mock.$tryParseEvmV1Calldata(binaryExtra, ethers.Typed.bool(true))).to.eventually.deep.equal(
+            [false, 0n, ethers.ZeroAddress],
+          );
+
           if (!address) {
             await expect(this.mock.$formatEvmV1(ethers.Typed.uint256(reference))).to.eventually.equal(
               binary.toLowerCase(),