chore: cherry pick multisig vote reset improvement (#87)

* feat: full test coverage (#85)

* feat: unit test coverage

* fix: slither

---------

Co-authored-by: Dean Amiel <[email protected]>

* feat(AxelarServiceGovernance): reset votes when approving multisig proposal

* docs(AxelarServiceGovernance): documenting reset votes on new proposal. (#86)

---------

Co-authored-by: Dean <[email protected]>
Co-authored-by: Dean Amiel <[email protected]>
Co-authored-by: Kiryl Yermakou <[email protected]>

DESIGN.md

@@ -66,10 +66,10 @@ The contract orchestrates four governance operations:
 
 - **Cancel TimeLock Proposal**: Again, similar to Interchain Governance, it cancels an existing governance proposal.
 
-- **Approve Multisig Proposal**: Enables multisig proposal approval, setting the approval status of the proposal to true and signaling successful approval via a `MultisigApproved` event.
+- **Approve Multisig Proposal**: This function enables multisig proposal approval by setting the proposal's approval status to true. It resets any previous voting and signals successful approval via a MultisigApproved event.
 
 - **Cancel Multisig Approval**: Cancels an approved multisig proposal, setting the approval status of the proposal to false and indicating successful cancellation through a `MultisigCancelled` event.
 
 ### Secure Execution of Multisig Proposals
 
-Upon receiving the necessary number of signatory approvals, a multisig proposal becomes eligible for execution. Before execution, the contract verifies the proposal's approval status; if the approval status is false, the transaction is reverted. Following successful execution, the proposal's approval status is reset, and a `MultisigExecuted` event is emitted.
+Each time a new multisig proposal receives approval from governance, the multisig voting count is reset to 0. This ensures that any previous votes on similar proposals will not affect the new proposal. When a multisig proposal gathers the required number of signatory approvals, it becomes ready for execution. Before execution, the contract verifies the proposal's approval status. If the status is set to false, the transaction is reverted. Once executed successfully, the approval status of the proposal is reset, and a MultisigExecuted event gets emitted.

contracts/governance/AxelarServiceGovernance.sol

@@ -109,6 +109,18 @@ contract AxelarServiceGovernance is InterchainGovernance, BaseMultisig, IAxelarS
         } else if (commandType == uint256(ServiceGovernanceCommand.ApproveMultisigProposal)) {
             multisigApprovals[proposalHash] = true;
 
+            // Reset all previous votes for this proposal
+            _resetSignerVotes(
+                keccak256(
+                    abi.encodeWithSelector(
+                        AxelarServiceGovernance.executeMultisigProposal.selector,
+                        target,
+                        callData,
+                        nativeValue
+                    )
+                )
+            );
+
             emit MultisigApproved(proposalHash, target, callData, nativeValue);
             return;
         } else if (commandType == uint256(ServiceGovernanceCommand.CancelMultisigApproval)) {

contracts/governance/BaseMultisig.sol

@@ -166,16 +166,30 @@ contract BaseMultisig is IBaseMultisig {
         }
 
         // Clear vote count and voted booleans.
+        _resetVoting(voting);
+
+        emit MultisigOperationExecuted(topic);
+
+        return true;
+    }
+
+    /**
+     * @dev Internal function to reset the votes for a topic
+     */
+    function _resetSignerVotes(bytes32 topic) internal {
+        _resetVoting(votingPerTopic[signerEpoch][topic]);
+    }
+
+    /**
+     * @dev Internal function to reset the votes for a topic
+     */
+    function _resetVoting(Voting storage voting) internal {
         delete voting.voteCount;
 
         uint256 count = signers.accounts.length;
 
         for (uint256 i; i < count; ++i) {
             delete voting.hasVoted[signers.accounts[i]];
         }
-
-        emit MultisigOperationExecuted(topic);
-
-        return true;
     }
 }

contracts/test/gmp/DestinationChainReceiver.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { AxelarExecutable } from '../../executable/AxelarExecutable.sol';
+
+contract DestinationChainReceiver is AxelarExecutable {
+    event Received(uint256 num);
+
+    constructor(address gatewayAddress) AxelarExecutable(gatewayAddress) {}
+
+    function _execute(
+        string calldata, /*sourceChain*/
+        string calldata, /*sourceAddress*/
+        bytes calldata payload
+    ) internal override {
+        uint256 num = abi.decode(payload, (uint256));
+        emit Received(num);
+    }
+}

contracts/test/gmp/SourceChainSender.sol

@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { IAxelarGateway } from '../../interfaces/IAxelarGateway.sol';
+
+contract SourceChainSender {
+    IAxelarGateway public gateway;
+    string public destinationChain;
+    string public executableAddress;
+
+    event Sent(uint256 num);
+
+    constructor(
+        address gateway_,
+        string memory destinationChain_,
+        string memory executableAddress_
+    ) {
+        gateway = IAxelarGateway(gateway_);
+        destinationChain = destinationChain_;
+        executableAddress = executableAddress_;
+    }
+
+    function send(uint256 num) external {
+        bytes memory payload = abi.encode(num);
+        gateway.callContract(destinationChain, executableAddress, payload);
+        emit Sent(num);
+    }
+}

contracts/test/governance/TestMultiSigBase.sol → contracts/test/governance/TestBaseMultiSig.sol

@@ -4,6 +4,10 @@ pragma solidity ^0.8.0;
 
 import { BaseMultisig } from '../../governance/BaseMultisig.sol';
 
-contract TestMultiSigBase is BaseMultisig {
+contract TestBaseMultiSig is BaseMultisig {
     constructor(address[] memory accounts, uint256 threshold) BaseMultisig(accounts, threshold) {}
+
+    function resetVotes(bytes32 topic) external {
+        _resetSignerVotes(topic);
+    }
 }

contracts/test/governance/TestInterchainGovernance.sol

@@ -19,4 +19,14 @@ contract TestInterchainGovernance is InterchainGovernance {
     ) external {
         _execute(sourceChain, sourceAddress, payload);
     }
+
+    function executeToken(
+        string calldata sourceChain,
+        string calldata sourceAddress,
+        bytes calldata payload,
+        string calldata tokenSymbol,
+        uint256 amount
+    ) external pure {
+        _executeWithToken(sourceChain, sourceAddress, payload, tokenSymbol, amount);
+    }
 }

contracts/test/LibsTest.sol → contracts/test/libs/LibsTest.sol

@@ -2,14 +2,16 @@
 
 pragma solidity ^0.8.0;
 
-import { StringToAddress, AddressToString } from '../libs/AddressString.sol';
-import { Bytes32ToString, StringToBytes32 } from '../libs/Bytes32String.sol';
+import { StringToAddress, AddressToString } from '../../libs/AddressString.sol';
+import { Bytes32ToString, StringToBytes32 } from '../../libs/Bytes32String.sol';
+import { SafeNativeTransfer } from '../../libs/SafeNativeTransfer.sol';
 
 contract LibsTest {
     using AddressToString for address;
     using StringToAddress for string;
     using Bytes32ToString for bytes32;
     using StringToBytes32 for string;
+    using SafeNativeTransfer for address;
 
     function addressToString(address address_) external pure returns (string memory) {
         return address_.toString();
@@ -26,4 +28,10 @@ contract LibsTest {
     function stringToBytes32(string calldata string_) external pure returns (bytes32) {
         return string_.toBytes32();
     }
+
+    function nativeTransfer(address receiver, uint256 amount) external {
+        receiver.safeNativeTransfer(amount);
+    }
+
+    receive() external payable {}
 }

contracts/test/libs/NoFallback.sol

@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+contract NoFallback {
+    // No fallback function
+}

contracts/test/upgradable/CallInitProxy.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { InitProxy } from '../../upgradable/InitProxy.sol';
+
+contract CallInitProxy is InitProxy {
+    function getContractId() external pure returns (bytes32) {
+        return contractId();
+    }
+}

contracts/test/upgradable/InvalidUpgradableTest.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { Upgradable } from '../../upgradable/Upgradable.sol';
+
+contract InvalidUpgradableTest is Upgradable {
+    function contractId() external pure override returns (bytes32) {
+        return keccak256('invalid');
+    }
+}

contracts/test/upgradable/TestFinalProxy.sol

@@ -13,4 +13,8 @@ contract TestFinalProxy is FinalProxy {
         if (FINAL_IMPLEMENTATION_SALT != bytes32(uint256(keccak256('final-implementation')) - 1))
             revert('invalid final salt');
     }
+
+    function contractId() internal pure override returns (bytes32) {
+        return keccak256('proxy-implementation');
+    }
 }

contracts/test/upgradable/TestFixedProxy.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { FixedProxy } from '../../upgradable/FixedProxy.sol';
+
+contract TestFixedProxy is FixedProxy {
+    constructor(address implementationAddress) FixedProxy(implementationAddress) {}
+
+    function contractId() internal pure override returns (bytes32) {
+        return keccak256('proxy-implementation');
+    }
+}

contracts/test/upgradable/TestInitProxy.sol

@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+
+import { InitProxy } from '../../upgradable/InitProxy.sol';
+
+contract TestInitProxy is InitProxy {
+    function contractId() internal pure override returns (bytes32) {
+        return keccak256('proxy-implementation');
+    }
+}

contracts/test/upgradable/UpgradableTest.sol

@@ -5,12 +5,18 @@ pragma solidity ^0.8.0;
 import { Upgradable } from '../../upgradable/Upgradable.sol';
 
 contract UpgradableTest is Upgradable {
+    uint256 public num;
+
     constructor() Upgradable() {
         if (_IMPLEMENTATION_SLOT != bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1)) {
             revert('invalid implementation slot');
         }
     }
 
+    function _setup(bytes calldata data) internal override {
+        num = abi.decode(data, (uint256));
+    }
+
     function contractId() external pure override returns (bytes32) {
         return keccak256('test');
     }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@axelar-network/axelar-gmp-sdk-solidity",
-  "version": "5.3.0",
+  "version": "5.3.1",
   "description": "Solidity GMP SDK and utilities provided by Axelar for cross-chain development",
   "main": "index.js",
   "scripts": {