Change signature format (#145)

Change the signature format for Crosschain Messaging Layer to match revised EEA specification. This PR has wide ranging breaking changes. Previous implementations will not work with this implementation as the signature format has changed. Unfortunately, the golang integration tests for GPACT no longer work. The issue is when the segment function call is executed. Despite extensive analysis, the defect has not been identified.
mshantanu · Oct 21, 2022 · 720911a · 720911a
1 parent 1709683
commit 720911a
Show file tree

Hide file tree

Showing 41 changed files with 1,288 additions and 680 deletions.
diff --git a/contracts/contracts/src/messaging/common/MessagingRegistrar.sol b/contracts/contracts/src/messaging/common/MessagingRegistrar.sol
@@ -16,8 +16,13 @@ pragma solidity >=0.8;
 
 import "../../common/EcdsaSignatureVerification.sol";
 import "../../openzeppelin/access/Ownable.sol";
+import "./SignatureEncoding.sol";
 
-contract MessagingRegistrar is EcdsaSignatureVerification, Ownable {
+contract MessagingRegistrar is
+    EcdsaSignatureVerification,
+    SignatureEncoding,
+    Ownable
+{
     struct BlockchainRecord {
         uint256 signingThreshold;
         uint256 numSigners;
@@ -90,62 +95,36 @@ contract MessagingRegistrar is EcdsaSignatureVerification, Ownable {
         );
     }
 
+    // Verify signatures and check the threshold
     function verifyAndCheckThreshold(
         uint256 _blockchainId,
-        address[] calldata _signers,
-        bytes32[] calldata _sigR,
-        bytes32[] calldata _sigS,
-        uint8[] calldata _sigV,
+        bytes calldata _signatures,
         bytes calldata _plainText
-    ) external view returns (bool) {
-        checkThreshold(_blockchainId, _signers);
-        return verify(_blockchainId, _signers, _sigR, _sigS, _sigV, _plainText);
+    ) external view {
+        Signatures memory sigs = decodeSignature(_signatures);
+
+        // Check signing threshold
+        require(
+            sigs.signatures.length >=
+                blockchains[_blockchainId].signingThreshold,
+            "Not enough signers"
+        );
+
+        checkUniqueSigners(sigs);
+        verifyInternal(_blockchainId, sigs, _plainText);
     }
 
+    // Version of verify called by Event Relay
+    // Verify a signature
     function verify(
         uint256 _blockchainId,
-        address[] calldata _signers,
-        bytes32[] calldata _sigR,
-        bytes32[] calldata _sigS,
-        uint8[] calldata _sigV,
+        bytes calldata _signatures,
         bytes calldata _plainText
     ) public view returns (bool) {
-        uint256 signersLength = _signers.length;
-        require(signersLength == _sigR.length, "sigR length mismatch");
-        require(signersLength == _sigS.length, "sigS length mismatch");
-        require(signersLength == _sigV.length, "sigV length mismatch");
+        Signatures memory sigs = decodeSignature(_signatures);
 
-        for (uint256 i = 0; i < signersLength; i++) {
-            // Check that signer is a signer for this blockchain
-            require(
-                blockchains[_blockchainId].signers[_signers[i]],
-                "Signer not signer for this blockchain"
-            );
-            // Verify the signature
-            require(
-                verifySigComponents(
-                    _signers[i],
-                    _plainText,
-                    _sigR[i],
-                    _sigS[i],
-                    _sigV[i]
-                ),
-                "Signature did not verify"
-            );
-        }
-        return true;
-    }
-
-    function checkThreshold(uint256 _blockchainId, address[] calldata _signers)
-        public
-        view
-        returns (bool)
-    {
-        uint256 signersLength = _signers.length;
-        require(
-            signersLength >= blockchains[_blockchainId].signingThreshold,
-            "Not enough signers"
-        );
+        checkUniqueSigners(sigs);
+        verifyInternal(_blockchainId, sigs, _plainText);
         return true;
     }
 
@@ -198,4 +177,42 @@ contract MessagingRegistrar is EcdsaSignatureVerification, Ownable {
         require(_newThreshold != 0, "Threshold can not be zero");
         blockchains[_bcId].signingThreshold = _newThreshold;
     }
+
+    /**
+     * Revert if there is a duplicate signer in the list.
+     */
+    function checkUniqueSigners(Signatures memory _sigs) private pure {
+        for (uint256 i = 0; i < _sigs.signatures.length - 1; i++) {
+            address signer = _sigs.signatures[i].by;
+
+            for (uint256 j = i + 1; j < _sigs.signatures.length; j++) {
+                require(signer != _sigs.signatures[j].by, "Duplicate signer");
+            }
+        }
+    }
+
+    function verifyInternal(
+        uint256 _blockchainId,
+        Signatures memory _sigs,
+        bytes calldata _plainText
+    ) private view {
+        for (uint256 i = 0; i < _sigs.signatures.length; i++) {
+            // Check that signer is a signer for this blockchain
+            require(
+                blockchains[_blockchainId].signers[_sigs.signatures[i].by],
+                "Signer not registered for this blockchain"
+            );
+            // Verify the signature
+            require(
+                verifySigComponents(
+                    _sigs.signatures[i].by,
+                    _plainText,
+                    _sigs.signatures[i].sigR,
+                    _sigs.signatures[i].sigS,
+                    _sigs.signatures[i].sigV
+                ),
+                "Signature did not verify"
+            );
+        }
+    }
 }
diff --git a/contracts/contracts/src/messaging/common/SignatureEncoding.sol b/contracts/contracts/src/messaging/common/SignatureEncoding.sol
@@ -0,0 +1,58 @@
+/*
+ * Copyright 2022 ConsenSys Software Inc
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+pragma solidity ^0.8;
+
+/**
+ * Signature encoding is defined in the EEA Crosschain Messaging specification:
+ * https://entethalliance.github.io/crosschain-interoperability/draft_crosschain_techspec_messaging.html
+ */
+contract SignatureEncoding {
+    uint16 constant ECDSA_SIGNATURE = 1;
+
+    struct Signature {
+        address by;
+        bytes32 sigR;
+        bytes32 sigS;
+        uint8 sigV;
+        bytes meta;
+    }
+
+    struct Signatures {
+        uint256 typ;
+        Signature[] signatures;
+    }
+
+    /**
+     * Decode a signature blob.
+     *
+     * @param _signatures Encoded signatures.
+     * @return Signture object.
+     */
+    function decodeSignature(bytes calldata _signatures)
+        internal
+        pure
+        returns (Signatures memory)
+    {
+        (
+            ,
+            /* Skip offset of dynamic type */
+            uint16 sigType
+        ) = abi.decode(_signatures, (uint256, uint16));
+        require(sigType == ECDSA_SIGNATURE, "Signature unknown type");
+
+        Signatures memory sigs = abi.decode(_signatures, (Signatures));
+        return sigs;
+    }
+}
diff --git a/contracts/contracts/src/messaging/common/SignatureEncodingTest.sol b/contracts/contracts/src/messaging/common/SignatureEncodingTest.sol
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2022 ConsenSys Software Inc
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+pragma solidity ^0.8;
+
+import "./SignatureEncoding.sol";
+
+/**
+ * Signature encoding is defined in the EEA Crosschain Messaging specification:
+ * https://entethalliance.github.io/crosschain-interoperability/draft_crosschain_techspec_messaging.html
+ */
+contract SignatureEncodingTest is SignatureEncoding {
+    event ShowSig(bytes encoded);
+
+    function testEmitSignatureBlob() external {
+        Signature memory sig1 = Signature(
+            address(0x21),
+            bytes32(uint256(0x22)),
+            bytes32(uint256(0x23)),
+            uint8(0x24),
+            bytes("abc")
+        );
+        Signature[] memory s = new Signature[](1);
+        s[0] = sig1;
+        Signatures memory sigs = Signatures(0x25, s);
+        bytes memory encoded = abi.encode(sigs);
+        emit ShowSig(encoded);
+    }
+
+    event SigType(uint256 typ);
+    event SigInfo1(uint256 typ, uint256 len);
+    event SigInfo2(
+        address by,
+        bytes32 sigR,
+        bytes32 sigS,
+        uint8 sigV,
+        bytes meta
+    );
+
+    function testDecodeSignatureType(bytes calldata _signatures) external {
+        (, uint256 sigType) = abi.decode(_signatures, (uint256, uint256));
+        emit SigType(sigType);
+    }
+
+    function testDecodeSignature(bytes calldata _signatures) external {
+        Signatures memory sigs = decodeSignature(_signatures);
+
+        emit SigInfo1(sigs.typ, sigs.signatures.length);
+
+        for (uint256 i = 0; i < sigs.signatures.length; i++) {
+            Signature memory sig = sigs.signatures[i];
+            emit SigInfo2(sig.by, sig.sigR, sig.sigS, sig.sigV, sig.meta);
+        }
+    }
+}
diff --git a/contracts/contracts/src/messaging/eventattest/EventAttestationVerifier.sol b/contracts/contracts/src/messaging/eventattest/EventAttestationVerifier.sol
@@ -16,60 +16,28 @@ pragma solidity >=0.8;
 
 import "../interface/CrosschainVerifier.sol";
 import "../common/MessagingRegistrar.sol";
-import "../../common/BytesUtil.sol";
+import "../common/SignatureEncoding.sol";
 
-contract EventAttestationVerifier is CrosschainVerifier, BytesUtil {
+contract EventAttestationVerifier is CrosschainVerifier, SignatureEncoding {
     MessagingRegistrar registrar;
 
     constructor(address _registrar) {
         registrar = MessagingRegistrar(_registrar);
     }
 
-    uint256 constant LEN_OF_LEN = 4;
-    uint256 constant LEN_OF_SIG = 20 + 32 + 32 + 1;
-
     function decodeAndVerifyEvent(
         uint256 _blockchainId,
         bytes32, /* _eventSig */
         bytes calldata _encodedEvent,
-        bytes calldata _signature
-    ) external view override {
-        address[] memory signers;
-        bytes32[] memory sigRs;
-        bytes32[] memory sigSs;
-        uint8[] memory sigVs;
-
-        {
-            uint32 len = BytesUtil.bytesToUint32(_signature, 0);
-            require(
-                _signature.length == LEN_OF_LEN + len * LEN_OF_SIG,
-                "Signature incorrect length"
-            );
-
-            signers = new address[](len);
-            sigRs = new bytes32[](len);
-            sigSs = new bytes32[](len);
-            sigVs = new uint8[](len);
-
-            uint256 offset = LEN_OF_LEN;
-            for (uint256 i = 0; i < len; i++) {
-                signers[i] = BytesUtil.bytesToAddress2(_signature, offset);
-                offset += 20;
-                sigRs[i] = BytesUtil.bytesToBytes32(_signature, offset);
-                offset += 32;
-                sigSs[i] = BytesUtil.bytesToBytes32(_signature, offset);
-                offset += 32;
-                sigVs[i] = BytesUtil.bytesToUint8(_signature, offset);
-                offset += 1;
-            }
-        }
+        bytes calldata _signatures
+    ) external view override returns (bool) {
         registrar.verifyAndCheckThreshold(
             _blockchainId,
-            signers,
-            sigRs,
-            sigSs,
-            sigVs,
+            _signatures,
             _encodedEvent
         );
+
+        // A return value is needed so that Web3J generates a wrapper for this function.
+        return true;
     }
 }