From 73d60a02a04fd742ababb1edf8114579f52f3594 Mon Sep 17 00:00:00 2001 From: Milap Sheth Date: Tue, 12 Sep 2023 19:54:31 -0400 Subject: [PATCH] chore: cherry pick multisig vote reset improvement (#87) * feat: full test coverage (#85) * feat: unit test coverage * fix: slither --------- Co-authored-by: Dean Amiel * feat(AxelarServiceGovernance): reset votes when approving multisig proposal * docs(AxelarServiceGovernance): documenting reset votes on new proposal. (#86) --------- Co-authored-by: Dean Co-authored-by: Dean Amiel Co-authored-by: Kiryl Yermakou --- DESIGN.md | 4 +- .../governance/AxelarServiceGovernance.sol | 12 + contracts/governance/BaseMultisig.sol | 22 +- .../test/gmp/DestinationChainReceiver.sol | 20 + contracts/test/gmp/SourceChainSender.sol | 29 + ...tMultiSigBase.sol => TestBaseMultiSig.sol} | 6 +- .../governance/TestInterchainGovernance.sol | 10 + contracts/test/{ => libs}/LibsTest.sol | 12 +- contracts/test/libs/NoFallback.sol | 7 + contracts/test/upgradable/CallInitProxy.sol | 11 + .../test/upgradable/InvalidUpgradableTest.sol | 11 + contracts/test/upgradable/TestFinalProxy.sol | 4 + contracts/test/upgradable/TestFixedProxy.sol | 13 + contracts/test/upgradable/TestInitProxy.sol | 11 + contracts/test/upgradable/UpgradableTest.sol | 6 + package-lock.json | 4 +- package.json | 2 +- test/GMP/GMP.js | 542 ++++++++++++------ test/GMP/GMPE.js | 203 +++++++ test/deploy/Create2Deployer.js | 38 ++ test/governance/AxelarServiceGovernance.js | 34 +- .../{MultisigBase.js => BaseMultisig.js} | 50 +- test/governance/InterchainGovernance.js | 12 + test/upgradable/Proxy.js | 154 ++++- test/upgradable/Upgradable.js | 83 +++ test/utils/Util.js | 59 ++ 26 files changed, 1161 insertions(+), 198 deletions(-) create mode 100644 contracts/test/gmp/DestinationChainReceiver.sol create mode 100644 contracts/test/gmp/SourceChainSender.sol rename contracts/test/governance/{TestMultiSigBase.sol => TestBaseMultiSig.sol} (62%) rename contracts/test/{ => libs}/LibsTest.sol (63%) create mode 100644 contracts/test/libs/NoFallback.sol create mode 100644 contracts/test/upgradable/CallInitProxy.sol create mode 100644 contracts/test/upgradable/InvalidUpgradableTest.sol create mode 100644 contracts/test/upgradable/TestFixedProxy.sol create mode 100644 contracts/test/upgradable/TestInitProxy.sol rename test/governance/{MultisigBase.js => BaseMultisig.js} (81%) diff --git a/DESIGN.md b/DESIGN.md index 31bf3978..befb175c 100644 --- a/DESIGN.md +++ b/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. diff --git a/contracts/governance/AxelarServiceGovernance.sol b/contracts/governance/AxelarServiceGovernance.sol index bcf0f8a7..8ef13793 100644 --- a/contracts/governance/AxelarServiceGovernance.sol +++ b/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)) { diff --git a/contracts/governance/BaseMultisig.sol b/contracts/governance/BaseMultisig.sol index d296f057..531fd466 100644 --- a/contracts/governance/BaseMultisig.sol +++ b/contracts/governance/BaseMultisig.sol @@ -166,6 +166,24 @@ 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; @@ -173,9 +191,5 @@ contract BaseMultisig is IBaseMultisig { for (uint256 i; i < count; ++i) { delete voting.hasVoted[signers.accounts[i]]; } - - emit MultisigOperationExecuted(topic); - - return true; } } diff --git a/contracts/test/gmp/DestinationChainReceiver.sol b/contracts/test/gmp/DestinationChainReceiver.sol new file mode 100644 index 00000000..9fefaff6 --- /dev/null +++ b/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); + } +} diff --git a/contracts/test/gmp/SourceChainSender.sol b/contracts/test/gmp/SourceChainSender.sol new file mode 100644 index 00000000..3ded096d --- /dev/null +++ b/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); + } +} diff --git a/contracts/test/governance/TestMultiSigBase.sol b/contracts/test/governance/TestBaseMultiSig.sol similarity index 62% rename from contracts/test/governance/TestMultiSigBase.sol rename to contracts/test/governance/TestBaseMultiSig.sol index 5c9afa2d..0a0c5c60 100644 --- a/contracts/test/governance/TestMultiSigBase.sol +++ b/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); + } } diff --git a/contracts/test/governance/TestInterchainGovernance.sol b/contracts/test/governance/TestInterchainGovernance.sol index 8da9e749..1d007a16 100644 --- a/contracts/test/governance/TestInterchainGovernance.sol +++ b/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); + } } diff --git a/contracts/test/LibsTest.sol b/contracts/test/libs/LibsTest.sol similarity index 63% rename from contracts/test/LibsTest.sol rename to contracts/test/libs/LibsTest.sol index 298b40e0..82745b4e 100644 --- a/contracts/test/LibsTest.sol +++ b/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 {} } diff --git a/contracts/test/libs/NoFallback.sol b/contracts/test/libs/NoFallback.sol new file mode 100644 index 00000000..50c3f2c5 --- /dev/null +++ b/contracts/test/libs/NoFallback.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +contract NoFallback { + // No fallback function +} diff --git a/contracts/test/upgradable/CallInitProxy.sol b/contracts/test/upgradable/CallInitProxy.sol new file mode 100644 index 00000000..e3d392c1 --- /dev/null +++ b/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(); + } +} diff --git a/contracts/test/upgradable/InvalidUpgradableTest.sol b/contracts/test/upgradable/InvalidUpgradableTest.sol new file mode 100644 index 00000000..34ecd7b9 --- /dev/null +++ b/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'); + } +} diff --git a/contracts/test/upgradable/TestFinalProxy.sol b/contracts/test/upgradable/TestFinalProxy.sol index afdaa1d7..a119823a 100644 --- a/contracts/test/upgradable/TestFinalProxy.sol +++ b/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'); + } } diff --git a/contracts/test/upgradable/TestFixedProxy.sol b/contracts/test/upgradable/TestFixedProxy.sol new file mode 100644 index 00000000..fb93c170 --- /dev/null +++ b/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'); + } +} diff --git a/contracts/test/upgradable/TestInitProxy.sol b/contracts/test/upgradable/TestInitProxy.sol new file mode 100644 index 00000000..9ad7d9a6 --- /dev/null +++ b/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'); + } +} diff --git a/contracts/test/upgradable/UpgradableTest.sol b/contracts/test/upgradable/UpgradableTest.sol index 06d43b91..cb047a2d 100644 --- a/contracts/test/upgradable/UpgradableTest.sol +++ b/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'); } diff --git a/package-lock.json b/package-lock.json index f5244b4a..510c5f76 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@axelar-network/axelar-gmp-sdk-solidity", - "version": "5.3.0", + "version": "5.3.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@axelar-network/axelar-gmp-sdk-solidity", - "version": "5.3.0", + "version": "5.3.1", "license": "MIT", "devDependencies": { "@axelar-network/axelar-contract-deployments": "github:axelarnetwork/axelar-contract-deployments#acfe0fb83cc20fee11206dacbb607f1c0b048434", diff --git a/package.json b/package.json index b187b844..f7f208f7 100644 --- a/package.json +++ b/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": { diff --git a/test/GMP/GMP.js b/test/GMP/GMP.js index 24fefa7a..f83c4cc5 100644 --- a/test/GMP/GMP.js +++ b/test/GMP/GMP.js @@ -23,6 +23,11 @@ describe('GMP', () => { let tokenA; let tokenB; + let sourceChainSenderFactory; + let sourceChainSender; + let destinationChainReceiverFactory; + let destinationChainReceiver; + let ownerWallet; let userWallet; @@ -34,6 +39,7 @@ describe('GMP', () => { const symbolB = 'testTokenY'; const decimals = 16; const capacity = 0; + const num = 10; before(async () => { [ownerWallet, userWallet] = await ethers.getSigners(); @@ -58,205 +64,391 @@ describe('GMP', () => { 'ERC20MintableBurnable', ownerWallet, ); - }); - - beforeEach(async () => { - sourceChainGateway = await gatewayFactory - .deploy() - .then((d) => d.deployed()); - destinationChainGateway = await gatewayFactory - .deploy() - .then((d) => d.deployed()); - - tokenA = await tokenFactory - .deploy(nameA, symbolA, decimals) - .then((d) => d.deployed()); - - tokenB = await tokenFactory - .deploy(nameB, symbolB, decimals) - .then((d) => d.deployed()); - - await sourceChainGateway.deployToken( - defaultAbiCoder.encode( - ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], - [nameA, symbolA, decimals, capacity, ethers.constants.AddressZero, 0], - ), - keccak256('0x'), - ); - await sourceChainGateway.deployToken( - defaultAbiCoder.encode( - ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], - [nameB, symbolB, decimals, capacity, ethers.constants.AddressZero, 0], - ), - keccak256('0x'), - ); - - await destinationChainGateway.deployToken( - defaultAbiCoder.encode( - ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], - [nameA, symbolA, decimals, capacity, tokenA.address, 0], - ), - keccak256('0x'), - ); - await destinationChainGateway.deployToken( - defaultAbiCoder.encode( - ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], - [nameB, symbolB, decimals, capacity, tokenB.address, 0], - ), - keccak256('0x'), + sourceChainSenderFactory = await ethers.getContractFactory( + 'SourceChainSender', + ownerWallet, ); - - destinationChainTokenSwapper = await destinationChainTokenSwapperFactory - .deploy(tokenA.address.toString(), tokenB.address.toString()) - .then((d) => d.deployed()); - - destinationChainSwapExecutable = await destinationChainSwapExecutableFactory - .deploy( - destinationChainGateway.address, - destinationChainTokenSwapper.address, - ) - .then((d) => d.deployed()); - - sourceChainSwapCaller = await sourceChainSwapCallerFactory - .deploy( - sourceChainGateway.address, - destinationChain, - destinationChainSwapExecutable.address, - ) - .then((d) => d.deployed()); - - await tokenA.mint(destinationChainGateway.address, 1e9); - await tokenB.mint(destinationChainTokenSwapper.address, 1e9); - - await sourceChainGateway.mintToken( - defaultAbiCoder.encode( - ['string', 'address', 'uint256'], - [symbolA, userWallet.address, 1e9], - keccak256('0x'), - ), - keccak256('0x'), + destinationChainReceiverFactory = await ethers.getContractFactory( + 'DestinationChainReceiver', + ownerWallet, ); - await ( - await tokenA.connect(ownerWallet).mint(userWallet.address, 1e9) - ).wait(); }); describe('AxelarExecutable', () => { - it('should swap tokens on remote chain', async () => { - const swapAmount = 1e6; - const convertedAmount = 2 * swapAmount; - const payload = defaultAbiCoder.encode( - ['string', 'string'], - [symbolB, userWallet.address.toString()], - ); - const payloadHash = keccak256(payload); - - const sourceChainTokenA = tokenFactory - .connect(userWallet) - .attach(await sourceChainGateway.tokenAddresses(symbolA)); - - await sourceChainTokenA.approve( - sourceChainSwapCaller.address, - swapAmount, - ); - - await expect( - sourceChainSwapCaller + describe('Call Contract with Token', () => { + beforeEach(async () => { + sourceChainGateway = await gatewayFactory + .deploy() + .then((d) => d.deployed()); + destinationChainGateway = await gatewayFactory + .deploy() + .then((d) => d.deployed()); + + tokenA = await tokenFactory + .deploy(nameA, symbolA, decimals) + .then((d) => d.deployed()); + + tokenB = await tokenFactory + .deploy(nameB, symbolB, decimals) + .then((d) => d.deployed()); + + await sourceChainGateway.deployToken( + defaultAbiCoder.encode( + ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], + [ + nameA, + symbolA, + decimals, + capacity, + ethers.constants.AddressZero, + 0, + ], + ), + keccak256('0x'), + ); + await sourceChainGateway.deployToken( + defaultAbiCoder.encode( + ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], + [ + nameB, + symbolB, + decimals, + capacity, + ethers.constants.AddressZero, + 0, + ], + ), + keccak256('0x'), + ); + + await destinationChainGateway.deployToken( + defaultAbiCoder.encode( + ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], + [nameA, symbolA, decimals, capacity, tokenA.address, 0], + ), + keccak256('0x'), + ); + await destinationChainGateway.deployToken( + defaultAbiCoder.encode( + ['string', 'string', 'uint8', 'uint256', ' address', 'uint256'], + [nameB, symbolB, decimals, capacity, tokenB.address, 0], + ), + keccak256('0x'), + ); + + destinationChainTokenSwapper = await destinationChainTokenSwapperFactory + .deploy(tokenA.address.toString(), tokenB.address.toString()) + .then((d) => d.deployed()); + + destinationChainSwapExecutable = + await destinationChainSwapExecutableFactory + .deploy( + destinationChainGateway.address, + destinationChainTokenSwapper.address, + ) + .then((d) => d.deployed()); + + sourceChainSwapCaller = await sourceChainSwapCallerFactory + .deploy( + sourceChainGateway.address, + destinationChain, + destinationChainSwapExecutable.address, + ) + .then((d) => d.deployed()); + + await tokenA.mint(destinationChainGateway.address, 1e9); + await tokenB.mint(destinationChainTokenSwapper.address, 1e9); + + await sourceChainGateway.mintToken( + defaultAbiCoder.encode( + ['string', 'address', 'uint256'], + [symbolA, userWallet.address, 1e9], + keccak256('0x'), + ), + keccak256('0x'), + ); + await ( + await tokenA.connect(ownerWallet).mint(userWallet.address, 1e9) + ).wait(); + }); + + it('should revert without gateway approval', async () => { + const swapAmount = 1e6; + const payload = defaultAbiCoder.encode( + ['string', 'string'], + [symbolB, userWallet.address.toString()], + ); + const payloadHash = keccak256(payload); + + const sourceChainTokenA = tokenFactory .connect(userWallet) - .swapToken(symbolA, symbolB, swapAmount, userWallet.address), - ) - .to.emit(sourceChainGateway, 'ContractCallWithToken') - .withArgs( + .attach(await sourceChainGateway.tokenAddresses(symbolA)); + + await sourceChainTokenA.approve( + sourceChainSwapCaller.address, + swapAmount, + ); + + await expect( + sourceChainSwapCaller + .connect(userWallet) + .swapToken(symbolA, symbolB, swapAmount, userWallet.address), + ) + .to.emit(sourceChainGateway, 'ContractCallWithToken') + .withArgs( + sourceChainSwapCaller.address.toString(), + destinationChain, + destinationChainSwapExecutable.address.toString(), + payloadHash, + payload, + symbolA, + swapAmount, + ); + + const approveCommandId = getRandomID(); + + const swap = destinationChainSwapExecutable.executeWithToken( + approveCommandId, + sourceChain, sourceChainSwapCaller.address.toString(), - destinationChain, - destinationChainSwapExecutable.address.toString(), - payloadHash, payload, symbolA, swapAmount, ); - const approveCommandId = getRandomID(); - const sourceTxHash = keccak256('0x123abc123abc'); - const sourceEventIndex = 17; - - const approveWithMintData = defaultAbiCoder.encode( - [ - 'string', - 'string', - 'address', - 'bytes32', - 'string', - 'uint256', - 'bytes32', - 'uint256', - ], - [ - sourceChain, + await expect(swap).to.be.revertedWithCustomError( + destinationChainSwapExecutable, + 'NotApprovedByGateway', + ); + }); + + it('should swap tokens on remote chain', async () => { + const swapAmount = 1e6; + const convertedAmount = 2 * swapAmount; + const payload = defaultAbiCoder.encode( + ['string', 'string'], + [symbolB, userWallet.address.toString()], + ); + const payloadHash = keccak256(payload); + + const sourceChainTokenA = tokenFactory + .connect(userWallet) + .attach(await sourceChainGateway.tokenAddresses(symbolA)); + + await sourceChainTokenA.approve( sourceChainSwapCaller.address, - destinationChainSwapExecutable.address, - payloadHash, - symbolA, swapAmount, - sourceTxHash, - sourceEventIndex, - ], - ); - - const approveExecute = - await destinationChainGateway.approveContractCallWithMint( - approveWithMintData, - approveCommandId, ); - await expect(approveExecute) - .to.emit(destinationChainGateway, 'ContractCallApprovedWithMint') - .withArgs( + await expect( + sourceChainSwapCaller + .connect(userWallet) + .swapToken(symbolA, symbolB, swapAmount, userWallet.address), + ) + .to.emit(sourceChainGateway, 'ContractCallWithToken') + .withArgs( + sourceChainSwapCaller.address.toString(), + destinationChain, + destinationChainSwapExecutable.address.toString(), + payloadHash, + payload, + symbolA, + swapAmount, + ); + const approveCommandId = getRandomID(); + const sourceTxHash = keccak256('0x123abc123abc'); + const sourceEventIndex = 17; + + const approveWithMintData = defaultAbiCoder.encode( + [ + 'string', + 'string', + 'address', + 'bytes32', + 'string', + 'uint256', + 'bytes32', + 'uint256', + ], + [ + sourceChain, + sourceChainSwapCaller.address, + destinationChainSwapExecutable.address, + payloadHash, + symbolA, + swapAmount, + sourceTxHash, + sourceEventIndex, + ], + ); + + const approveExecute = + await destinationChainGateway.approveContractCallWithMint( + approveWithMintData, + approveCommandId, + ); + + await expect(approveExecute) + .to.emit(destinationChainGateway, 'ContractCallApprovedWithMint') + .withArgs( + approveCommandId, + sourceChain, + sourceChainSwapCaller.address.toString(), + destinationChainSwapExecutable.address, + payloadHash, + symbolA, + swapAmount, + sourceTxHash, + sourceEventIndex, + ); + + const swap = destinationChainSwapExecutable.executeWithToken( approveCommandId, sourceChain, sourceChainSwapCaller.address.toString(), - destinationChainSwapExecutable.address, - payloadHash, + payload, symbolA, swapAmount, - sourceTxHash, - sourceEventIndex, ); + await expect(swap) + .to.emit(tokenA, 'Transfer') + .withArgs( + destinationChainGateway.address, + destinationChainSwapExecutable.address, + swapAmount, + ) + .and.to.emit(tokenB, 'Transfer') + .withArgs( + destinationChainTokenSwapper.address, + destinationChainSwapExecutable.address, + convertedAmount, + ) + .and.to.emit(tokenB, 'Transfer') + .withArgs( + destinationChainSwapExecutable.address, + destinationChainGateway.address, + convertedAmount, + ) + .and.to.emit(destinationChainGateway, 'TokenSent') + .withArgs( + destinationChainSwapExecutable.address, + sourceChain, + userWallet.address.toString(), + symbolB, + convertedAmount, + ); + }); + }); - const swap = destinationChainSwapExecutable.executeWithToken( - approveCommandId, - sourceChain, - sourceChainSwapCaller.address.toString(), - payload, - symbolA, - swapAmount, - ); - await expect(swap) - .to.emit(tokenA, 'Transfer') - .withArgs( - destinationChainGateway.address, - destinationChainSwapExecutable.address, - swapAmount, - ) - .and.to.emit(tokenB, 'Transfer') - .withArgs( - destinationChainTokenSwapper.address, - destinationChainSwapExecutable.address, - convertedAmount, - ) - .and.to.emit(tokenB, 'Transfer') - .withArgs( - destinationChainSwapExecutable.address, - destinationChainGateway.address, - convertedAmount, - ) - .and.to.emit(destinationChainGateway, 'TokenSent') - .withArgs( - destinationChainSwapExecutable.address, + describe('Call Contract', () => { + beforeEach(async () => { + sourceChainGateway = await gatewayFactory + .deploy() + .then((d) => d.deployed()); + destinationChainGateway = await gatewayFactory + .deploy() + .then((d) => d.deployed()); + + destinationChainReceiver = await destinationChainReceiverFactory + .deploy(destinationChainGateway.address) + .then((d) => d.deployed()); + + sourceChainSender = await sourceChainSenderFactory + .deploy( + sourceChainGateway.address, + destinationChain, + destinationChainReceiver.address, + ) + .then((d) => d.deployed()); + }); + + it('should revert without gateway approval', async () => { + const payload = defaultAbiCoder.encode(['uint256'], [num]); + const payloadHash = keccak256(payload); + + await expect(sourceChainSender.connect(userWallet).send(num)) + .to.emit(sourceChainGateway, 'ContractCall') + .withArgs( + sourceChainSender.address.toString(), + destinationChain, + destinationChainReceiver.address.toString(), + payloadHash, + payload, + ); + + const approveCommandId = getRandomID(); + + const receive = destinationChainReceiver.execute( + approveCommandId, + sourceChain, + sourceChainSender.address.toString(), + payload, + ); + + await expect(receive).to.be.revertedWithCustomError( + destinationChainReceiver, + 'NotApprovedByGateway', + ); + }); + + it('should call contract on another chain', async () => { + const payload = defaultAbiCoder.encode(['uint256'], [num]); + const payloadHash = keccak256(payload); + + await expect(sourceChainSender.connect(userWallet).send(num)) + .to.emit(sourceChainGateway, 'ContractCall') + .withArgs( + sourceChainSender.address.toString(), + destinationChain, + destinationChainReceiver.address.toString(), + payloadHash, + payload, + ); + + const approveCommandId = getRandomID(); + const sourceTxHash = keccak256('0x123abc123abc'); + const sourceEventIndex = 17; + + const approveData = defaultAbiCoder.encode( + ['string', 'string', 'address', 'bytes32', 'bytes32', 'uint256'], + [ + sourceChain, + sourceChainSender.address, + destinationChainReceiver.address, + payloadHash, + sourceTxHash, + sourceEventIndex, + ], + ); + + const approveExecute = + await destinationChainGateway.approveContractCall( + approveData, + approveCommandId, + ); + + await expect(approveExecute) + .to.emit(destinationChainGateway, 'ContractCallApproved') + .withArgs( + approveCommandId, + sourceChain, + sourceChainSender.address.toString(), + destinationChainReceiver.address, + payloadHash, + sourceTxHash, + sourceEventIndex, + ); + + const receive = destinationChainReceiver.execute( + approveCommandId, sourceChain, - userWallet.address.toString(), - symbolB, - convertedAmount, + sourceChainSender.address.toString(), + payload, ); + + await expect(receive) + .to.emit(destinationChainReceiver, 'Received') + .withArgs(num); + }); }); }); }); diff --git a/test/GMP/GMPE.js b/test/GMP/GMPE.js index 320b7164..472216a3 100644 --- a/test/GMP/GMPE.js +++ b/test/GMP/GMPE.js @@ -163,6 +163,12 @@ describe('GMPE', async () => { }); describe('AxelarExpressExecutable', () => { + it('should revert on deployment with invalid gateway address', async () => { + await expect( + expressExecutableTestFactory.deploy(AddressZero), + ).to.be.revertedWithCustomError(expressExecutableTest, 'InvalidAddress'); + }); + it('should expressCallWithToken a swap on remote chain', async () => { const swapAmount = 1e6; const convertedAmount = 2 * swapAmount; @@ -381,6 +387,19 @@ describe('GMPE', async () => { ); } + async function executionFailure() { + const executeTx = contract.execute( + commandId, + sourceChain, + sourceAddress, + payload, + ); + await expect(executeTx).to.be.revertedWithCustomError( + contract, + 'NotApprovedByGateway', + ); + } + async function expressFailure() { const expressTx = contract.expressExecute( commandId, @@ -435,6 +454,10 @@ describe('GMPE', async () => { await execution(); }); + it('Should not Execute without gateway approval', async () => { + await executionFailure(); + }); + it('Should Execute with express before approval', async () => { await expressSuccess(); await approve(); @@ -604,6 +627,21 @@ describe('GMPE', async () => { .withArgs(sourceChain, sourceAddress, payload, tokenSymbol, amount); } + async function executionFailed() { + const executeTx = contract.executeWithToken( + commandId, + sourceChain, + sourceAddress, + payload, + tokenSymbol, + amount, + ); + await expect(executeTx).to.be.revertedWithCustomError( + contract, + 'NotApprovedByGateway', + ); + } + async function expressFullfill() { const executeTx = contract.executeWithToken( commandId, @@ -639,6 +677,10 @@ describe('GMPE', async () => { await execution(); }); + it('Should not Execute without gateway approval', async () => { + await executionFailed(); + }); + it('Should Execute with express before approval', async () => { await expressSuccess(); await approve(); @@ -772,6 +814,19 @@ describe('GMPE', async () => { .withArgs(sourceChain, sourceAddress, payload); } + async function executionFailure() { + const executeTx = contract.execute( + commandId, + sourceChain, + sourceAddress, + payload, + ); + await expect(executeTx).to.be.revertedWithCustomError( + contract, + 'NotApprovedByGateway', + ); + } + async function expressFullfill() { const balance = BigInt( await ownerWallet.provider.getBalance(ownerWallet.address), @@ -802,16 +857,74 @@ describe('GMPE', async () => { await valuedAxelarExpressExecutableTest.setExpressToken(AddressZero); }); + it('should revert on deployment with invalid gateway address', async () => { + await expect( + valuedAxelarExpressExecutableTestFactory.deploy(AddressZero), + ).to.be.revertedWithCustomError( + valuedAxelarExpressExecutableTest, + 'InvalidAddress', + ); + }); + it('Should Execute without express', async () => { await approve(); await execution(); }); + + it('Should not Execute without gateway approval', async () => { + await executionFailure(); + }); + it('Should Execute with express before approval', async () => { await expressSuccess(); await approve(); await expressFullfill(); }); + it('Should return the correct express executor slot', async () => { + await expressSuccess(); + + const slotPrefix = + '0x2a41fec9a0df4e0996b975f71622c7164b0f652ea69d9dbcd6b24e81b20ab5e5'; + + const predictedSlot = keccak256( + defaultAbiCoder.encode( + ['bytes32', 'bytes32', 'string', 'string', 'bytes32'], + [slotPrefix, commandId, sourceChain, sourceAddress, payloadHash], + ), + ); + + const valueAtSlot = await ethers.provider.getStorageAt( + contract.address, + predictedSlot, + ); + + const predictedExecutorAddress = '0x' + valueAtSlot.slice(-40); + + const executorAddress = await contract.getExpressExecutor( + commandId, + sourceChain, + sourceAddress, + payloadHash, + ); + + expect(predictedExecutorAddress.toLowerCase()).to.eq( + executorAddress.toLowerCase(), + ); + + await approve(); + await expressFullfill(); + + const fullfilledExecutorAddress = await contract.getExpressExecutor( + commandId, + sourceChain, + sourceAddress, + payloadHash, + ); + + expect(fullfilledExecutorAddress).to.eq(AddressZero); + }); + it('Should not Execute with express before approval without proper payment', async () => { await expressNotPayed(); }); @@ -981,6 +1094,12 @@ describe('GMPE', async () => { await expressFullfill(); }); + it('Should Execute with express before approval with zero token value', async () => { + await expressSuccess(0); + await approve(); + await expressFullfill(0); + }); + it('Should not Execute with express before approval without proper payment', async () => { await expressNotPayed(); }); @@ -1205,6 +1324,71 @@ describe('GMPE', async () => { await expressFullfill(); }); + it('Should return the correct express executor with token slot', async () => { + await expressSuccess(); + + const slotPrefix = + '0xebf4535caee8019297b7be3ed867db0d00b69fedcdda98c5e2c41ea6e41a98d5'; + + const predictedSlot = keccak256( + defaultAbiCoder.encode( + [ + 'bytes32', + 'bytes32', + 'string', + 'string', + 'bytes32', + 'string', + 'uint256', + ], + [ + slotPrefix, + commandId, + sourceChain, + sourceAddress, + payloadHash, + tokenSymbol, + amount, + ], + ), + ); + + const valueAtSlot = await ethers.provider.getStorageAt( + contract.address, + predictedSlot, + ); + + const predictedExecutorAddress = '0x' + valueAtSlot.slice(-40); + + const executorAddress = await contract.getExpressExecutorWithToken( + commandId, + sourceChain, + sourceAddress, + payloadHash, + tokenSymbol, + amount, + ); + + expect(predictedExecutorAddress.toLowerCase()).to.eq( + executorAddress.toLowerCase(), + ); + + await approve(); + await expressFullfill(); + + const fullfilledExecutorAddress = + await contract.getExpressExecutorWithToken( + commandId, + sourceChain, + sourceAddress, + payloadHash, + tokenSymbol, + amount, + ); + + expect(fullfilledExecutorAddress).to.eq(AddressZero); + }); + it('Should not Execute with express before approval without proper payment', async () => { await expressNotPayed(); }); @@ -1386,6 +1570,21 @@ describe('GMPE', async () => { .withArgs(sourceChain, sourceAddress, payload, tokenSymbol, amount); } + async function executionFailure() { + const executeTx = contract.executeWithToken( + commandId, + sourceChain, + sourceAddress, + payload, + tokenSymbol, + amount, + ); + await expect(executeTx).to.be.revertedWithCustomError( + contract, + 'NotApprovedByGateway', + ); + } + async function expressFullfill() { const executeTx = contract .connect(userWallet) @@ -1427,6 +1626,10 @@ describe('GMPE', async () => { await execution(); }); + it('Should not Execute without gateway approval', async () => { + await executionFailure(); + }); + it('Should Execute with express before approval', async () => { await expressSuccess(); await approve(); diff --git a/test/deploy/Create2Deployer.js b/test/deploy/Create2Deployer.js index a4c4df71..27fdcd69 100644 --- a/test/deploy/Create2Deployer.js +++ b/test/deploy/Create2Deployer.js @@ -5,6 +5,7 @@ const { ethers } = require('hardhat'); const { expect } = chai; const { utils: { keccak256 }, + ContractFactory, } = ethers; const { create2DeployContract, @@ -150,6 +151,43 @@ describe('Create2Deployer', () => { expect(addresses[0]).to.not.equal(addresses[1]); }); + + it('should revert when deployed twice with the same salt', async () => { + const key = 'a test key'; + const address = await getCreate2Address( + deployer, + userWallet, + BurnableMintableCappedERC20, + key, + [name, symbol, decimals], + ); + + const contract = await create2DeployContract( + deployer, + userWallet, + BurnableMintableCappedERC20, + key, + [name, symbol, decimals], + ); + expect(contract.address).to.equal(address); + + const deployerContract = deployerFactory.attach(deployer); + + const salt = getSaltFromKey(key); + const factory = new ContractFactory( + BurnableMintableCappedERC20.abi, + BurnableMintableCappedERC20.bytecode, + ); + const bytecode = factory.getDeployTransaction( + name, + symbol, + decimals, + ).data; + + await expect( + deployerContract.connect(userWallet).deploy(bytecode, salt), + ).to.be.revertedWithCustomError(deployerContract, 'AlreadyDeployed'); + }); }); describe('deployAndInit', () => { diff --git a/test/governance/AxelarServiceGovernance.js b/test/governance/AxelarServiceGovernance.js index ab57b7b5..df8245bc 100644 --- a/test/governance/AxelarServiceGovernance.js +++ b/test/governance/AxelarServiceGovernance.js @@ -3,7 +3,7 @@ const chai = require('chai'); const { ethers } = require('hardhat'); const { - utils: { defaultAbiCoder, Interface }, + utils: { defaultAbiCoder, Interface, keccak256 }, } = ethers; const { expect } = chai; const { getPayloadAndProposalHash } = require('../utils'); @@ -398,6 +398,31 @@ describe('AxelarServiceGovernance', () => { calldata, ); + const msgData = serviceGovernance.interface.encodeFunctionData( + 'executeMultisigProposal', + [target, calldata, nativeValue], + ); + const msgDataHash = keccak256(msgData); + + expect(await serviceGovernance.getSignerVotesCount(msgDataHash)).to.equal( + 0, + ); + expect( + await serviceGovernance.hasSignerVoted(signer1.address, msgDataHash), + ).to.equal(false); + + await serviceGovernance + .connect(signer1) + .executeMultisigProposal(target, calldata, nativeValue) + .then((tx) => tx.wait()); + + expect(await serviceGovernance.getSignerVotesCount(msgDataHash)).to.equal( + 1, + ); + expect( + await serviceGovernance.hasSignerVoted(signer1.address, msgDataHash), + ).to.equal(true); + await expect( serviceGovernance.executeProposalAction( governanceChain, @@ -408,6 +433,13 @@ describe('AxelarServiceGovernance', () => { .to.emit(serviceGovernance, 'MultisigApproved') .withArgs(proposalHash, target, calldata, nativeValue); + expect(await serviceGovernance.getSignerVotesCount(msgDataHash)).to.equal( + 0, + ); + expect( + await serviceGovernance.hasSignerVoted(signer1.address, msgDataHash), + ).to.equal(false); + await serviceGovernance .connect(signer1) .executeMultisigProposal(target, calldata, nativeValue) diff --git a/test/governance/MultisigBase.js b/test/governance/BaseMultisig.js similarity index 81% rename from test/governance/MultisigBase.js rename to test/governance/BaseMultisig.js index 9eddf46c..c971ac0b 100644 --- a/test/governance/MultisigBase.js +++ b/test/governance/BaseMultisig.js @@ -6,7 +6,7 @@ const { } = ethers; const { expect } = chai; -describe('MultisigBase', () => { +describe('BaseMultisig', () => { let signer1, signer2, signer3, signer4, signer5, signer6, nonSigner; let initAccounts; let rotatedAccounts; @@ -23,7 +23,7 @@ describe('MultisigBase', () => { ); multiSigFactory = await ethers.getContractFactory( - 'TestMultiSigBase', + 'TestBaseMultiSig', signer1, ); }); @@ -116,6 +116,52 @@ describe('MultisigBase', () => { .withArgs(msgDataHash); }); + it('should reset the votes internally', async () => { + const newThreshold = 2; + + const rotateInterface = new Interface([ + 'function rotateSigners(address[] memory newAccounts, uint256 newThreshold) external payable', + ]); + const msgData = rotateInterface.encodeFunctionData('rotateSigners', [ + rotatedAccounts, + newThreshold, + ]); + const msgDataHash = keccak256(msgData); + + expect(await multiSig.getSignerVotesCount(msgDataHash)).to.equal(0); + expect( + await multiSig.hasSignerVoted(signer1.address, msgDataHash), + ).to.equal(false); + + await multiSig + .connect(signer1) + .rotateSigners(rotatedAccounts, newThreshold) + .then((tx) => tx.wait()); + + expect(await multiSig.getSignerVotesCount(msgDataHash)).to.equal(1); + expect( + await multiSig.hasSignerVoted(signer1.address, msgDataHash), + ).to.equal(true); + + await multiSig.resetVotes(msgDataHash); + + expect(await multiSig.getSignerVotesCount(msgDataHash)).to.equal(0); + expect( + await multiSig.hasSignerVoted(signer1.address, msgDataHash), + ).to.equal(false); + + await multiSig + .connect(signer1) + .rotateSigners(rotatedAccounts, newThreshold) + .then((tx) => tx.wait()); + + await expect( + multiSig.connect(signer2).rotateSigners(rotatedAccounts, newThreshold), + ) + .to.emit(multiSig, 'MultisigOperationExecuted') + .withArgs(msgDataHash); + }); + it('should revert on rotate signers if new threshold is too large', async () => { const newThreshold = 4; diff --git a/test/governance/InterchainGovernance.js b/test/governance/InterchainGovernance.js index a22e59c8..1a8e9f8b 100644 --- a/test/governance/InterchainGovernance.js +++ b/test/governance/InterchainGovernance.js @@ -479,4 +479,16 @@ describe('InterchainGovernance', () => { ), ).to.be.revertedWithCustomError(interchainGovernance, 'ExecutionFailed'); }); + + it('should revert on execute with token', async () => { + await expect( + interchainGovernance.executeToken( + governanceChain, + AddressZero, + '0x', + 'abc', + 123, + ), + ).to.be.revertedWithCustomError(interchainGovernance, 'TokenNotSupported'); + }); }); diff --git a/test/upgradable/Proxy.js b/test/upgradable/Proxy.js index d3591697..c4df4c00 100644 --- a/test/upgradable/Proxy.js +++ b/test/upgradable/Proxy.js @@ -3,6 +3,7 @@ const chai = require('chai'); const { utils: { defaultAbiCoder, keccak256 }, + constants: { AddressZero }, } = require('ethers'); const { expect } = chai; const { ethers } = require('hardhat'); @@ -14,6 +15,9 @@ describe('Proxy', async () => { let proxyImplementationFactory; let proxyImplementation; + let invalidProxyImplementationFactory; + let invalidProxyImplementation; + before(async () => { [owner, user] = await ethers.getSigners(); @@ -21,6 +25,11 @@ describe('Proxy', async () => { 'ProxyImplementation', owner, ); + + invalidProxyImplementationFactory = await ethers.getContractFactory( + 'InvalidProxyImplementation', + owner, + ); }); describe('FixedProxy', async () => { @@ -45,6 +54,20 @@ describe('Proxy', async () => { .then((d) => d.deployed()); }); + it('should revert with invalid contract id', async () => { + fixedProxyFactory = await ethers.getContractFactory( + 'TestFixedProxy', + owner, + ); + const invalidProxyImplementation = await invalidProxyImplementationFactory + .deploy() + .then((d) => d.deployed()); + + await expect( + fixedProxyFactory.deploy(invalidProxyImplementation.address), + ).to.be.revertedWithCustomError(fixedProxy, 'InvalidImplementation'); + }); + it('call to proxy invokes correct function in implementation', async () => { const input = 123; @@ -81,30 +104,27 @@ describe('Proxy', async () => { describe('Proxy & BaseProxy', async () => { let proxyFactory; - let invalidProxyImplementationFactory; let invalidSetupProxyImplementationFactory; let proxy; - let invalidProxyImplementation; let invalidSetupProxyImplementation; beforeEach(async () => { proxyFactory = await ethers.getContractFactory('TestProxy', owner); - invalidProxyImplementationFactory = await ethers.getContractFactory( - 'InvalidProxyImplementation', - owner, - ); + invalidSetupProxyImplementationFactory = await ethers.getContractFactory( 'InvalidSetupProxyImplementation', owner, ); - proxyImplementation = await proxyImplementationFactory + invalidProxyImplementation = await invalidProxyImplementationFactory .deploy() .then((d) => d.deployed()); - invalidProxyImplementation = await invalidProxyImplementationFactory + + proxyImplementation = await proxyImplementationFactory .deploy() .then((d) => d.deployed()); + invalidSetupProxyImplementation = await invalidSetupProxyImplementationFactory .deploy() @@ -190,6 +210,30 @@ describe('Proxy', async () => { expect(val).to.equal(input); }); + it('should shadow a call to setup on the implementation', async () => { + let setupParams = defaultAbiCoder.encode( + ['uint256', 'string'], + [123, 'test'], + ); + + proxy = await proxyFactory + .deploy(proxyImplementation.address, owner.address, setupParams) + .then((d) => d.deployed()); + + const implementationAsProxy = proxyImplementation.attach(proxy.address); + + setupParams = defaultAbiCoder.encode( + ['uint256', 'string'], + [456, 'test'], + ); + + await proxyImplementation.setup(setupParams); + + const val = await implementationAsProxy.value(); + + expect(val).to.eq(123); + }); + it('should preserve the proxy bytecode [ @skip-on-coverage ]', async () => { const proxyFactory = await ethers.getContractFactory('Proxy', owner); const proxyBytecode = proxyFactory.bytecode; @@ -230,6 +274,11 @@ describe('Proxy', async () => { proxyImplementation = await proxyImplementationFactory .deploy() .then((d) => d.deployed()); + + invalidProxyImplementation = await invalidProxyImplementationFactory + .deploy() + .then((d) => d.deployed()); + differentProxyImplementation = await differentProxyImplementationFactory .deploy() .then((d) => d.deployed()); @@ -294,6 +343,25 @@ describe('Proxy', async () => { ).to.be.revertedWithCustomError(finalProxy, 'SetupFailed'); }); + it('should revert on final upgrade with invalid contract ID', async () => { + const setupParams = '0x'; + const testFinalProxyFactory = await ethers.getContractFactory( + 'TestFinalProxy', + owner, + ); + + const finalProxy = await testFinalProxyFactory + .deploy(proxyImplementation.address, owner.address, setupParams) + .then((d) => d.deployed()); + + const bytecode = + await invalidProxyImplementationFactory.getDeployTransaction().data; + + await expect( + finalProxy.finalUpgrade(bytecode, setupParams), + ).to.be.revertedWithCustomError(finalProxy, 'InvalidImplementation'); + }); + it('after finalUpgrade, isFinal returns true', async () => { const setupParams = '0x'; @@ -311,6 +379,53 @@ describe('Proxy', async () => { expect(isFinal).to.be.true; }); + it('should perform final upgrade with setup params', async () => { + let setupParams = '0x'; + + finalProxy = await finalProxyFactory + .deploy( + differentProxyImplementation.address, + owner.address, + setupParams, + ) + .then((d) => d.deployed()); + + const bytecode = await proxyImplementationFactory.getDeployTransaction() + .data; + + setupParams = defaultAbiCoder.encode( + ['uint256', 'string'], + [123, 'test'], + ); + + await finalProxy.finalUpgrade(bytecode, setupParams); + + const isFinal = await finalProxy.isFinal(); + + expect(isFinal).to.be.true; + }); + + it('should return the correct implementation address', async () => { + const setupParams = '0x'; + + finalProxy = await finalProxyFactory + .deploy(proxyImplementation.address, owner.address, setupParams) + .then((d) => d.deployed()); + + let implementationAddress = await finalProxy.implementation(); + + expect(implementationAddress).to.eq(proxyImplementation.address); + + const bytecode = + await differentProxyImplementationFactory.getDeployTransaction().data; + + await finalProxy.finalUpgrade(bytecode, setupParams); + + implementationAddress = await finalProxy.implementation(); + + expect(implementationAddress).to.not.eq(AddressZero); + }); + it('reverts on second call to finalUpgrade', async () => { const setupParams = '0x'; @@ -399,6 +514,29 @@ describe('Proxy', async () => { ).to.be.revertedWithCustomError(initProxy, 'SetupFailed'); }); + it('should revert with invalid contract ID', async () => { + initProxyFactory = await ethers.getContractFactory( + 'TestInitProxy', + owner, + ); + + initProxy = await initProxyFactory.deploy().then((d) => d.deployed()); + + const invalidProxyImplementation = await invalidProxyImplementationFactory + .deploy() + .then((d) => d.deployed()); + + const setupParams = '0x'; + + await expect( + initProxy.init( + invalidProxyImplementation.address, + owner.address, + setupParams, + ), + ).to.be.revertedWithCustomError(initProxy, 'InvalidImplementation'); + }); + it('should initialize init proxy with setup params', async () => { const initVal = 123; const initName = 'test'; diff --git a/test/upgradable/Upgradable.js b/test/upgradable/Upgradable.js index d0acad9d..8dc7bca9 100644 --- a/test/upgradable/Upgradable.js +++ b/test/upgradable/Upgradable.js @@ -3,6 +3,10 @@ const chai = require('chai'); const { expect } = chai; const { ethers } = require('hardhat'); +const { + utils: { keccak256, defaultAbiCoder }, + constants: { AddressZero }, +} = ethers; const { deployCreate3Upgradable, upgradeUpgradable } = require('../../index'); const Proxy = require('../../artifacts/contracts/test/upgradable/ProxyTest.sol/ProxyTest.json'); @@ -44,6 +48,53 @@ describe('Upgradable', () => { ); }); + it('should store implementation address in the proxy, not the implementation', async () => { + const implementationAddress = await upgradable.implementation(); + + const implementation = upgradableTestFactory.attach(implementationAddress); + + expect(await implementation.implementation()).to.eq(AddressZero); + }); + + it('should revert on upgrade if called by non-owner', async () => { + await expect( + upgradable.connect(userWallet).upgrade(AddressZero, keccak256(0), '0x'), + ).to.be.revertedWithCustomError(upgradable, 'NotOwner'); + }); + + it('should revert on upgrade with invalid contract ID', async () => { + const invalidUpgradableTestFactory = await ethers.getContractFactory( + 'InvalidUpgradableTest', + ownerWallet, + ); + + const invalidUpgradableTest = await invalidUpgradableTestFactory + .deploy() + .then((d) => d.deployed()); + + const implementationCode = await ethers.provider.getCode( + invalidUpgradableTest.address, + ); + + const implementationCodeHash = keccak256(implementationCode); + + await expect( + upgradable.upgrade( + invalidUpgradableTest.address, + implementationCodeHash, + '0x', + ), + ).to.be.revertedWithCustomError(upgradable, 'InvalidImplementation'); + }); + + it('should revert on upgrade with invalid code hash', async () => { + const invalidCodeHash = keccak256(0); + + await expect( + upgradable.upgrade(upgradable.address, invalidCodeHash, '0x'), + ).to.be.revertedWithCustomError(upgradable, 'InvalidCodeHash'); + }); + it('should upgrade to a new implementation', async () => { const oldImplementation = await upgradable.implementation(); @@ -54,6 +105,38 @@ describe('Upgradable', () => { expect(newImplementation).not.to.be.equal(oldImplementation); }); + it('should upgrade to a new implementation with setup params', async () => { + const oldImplementation = await upgradable.implementation(); + + const setupParams = defaultAbiCoder.encode(['uint256'], [10]); + + await upgradeUpgradable( + upgradable.address, + ownerWallet, + Upgradable, + [], + setupParams, + ); + + const newImplementation = await upgradable.implementation(); + + expect(newImplementation).not.to.be.equal(oldImplementation); + }); + + it('should revert on upgrade if setup fails', async () => { + const implementation = await upgradable.implementation(); + + const setupParams = '0x00'; + + const implementationCode = await ethers.provider.getCode(implementation); + + const implementationCodeHash = keccak256(implementationCode); + + await expect( + upgradable.upgrade(implementation, implementationCodeHash, setupParams), + ).to.be.revertedWithCustomError(upgradable, 'SetupFailed'); + }); + it('should transfer ownership', async () => { await upgradable.connect(ownerWallet).transferOwnership(userWallet.address); diff --git a/test/utils/Util.js b/test/utils/Util.js index b18bb124..53eaeb85 100644 --- a/test/utils/Util.js +++ b/test/utils/Util.js @@ -8,16 +8,26 @@ describe('LibsTest', () => { let libsTestFactory; let libsTest; + let noFallBackFactory; + let noFallBack; + let ownerWallet; before(async () => { [ownerWallet] = await ethers.getSigners(); libsTestFactory = await ethers.getContractFactory('LibsTest', ownerWallet); + + noFallBackFactory = await ethers.getContractFactory( + 'NoFallback', + ownerWallet, + ); }); beforeEach(async () => { libsTest = await libsTestFactory.deploy().then((d) => d.deployed()); + + noFallBack = await noFallBackFactory.deploy().then((d) => d.deployed()); }); it('should convert address to lowercase string', async () => { @@ -38,6 +48,28 @@ describe('LibsTest', () => { ).to.equal(address); }); + it('should revert on invalid string to address conversion', async () => { + let invalidAddressString = '0x123'; + await expect( + libsTest.stringToAddress(invalidAddressString), + ).to.be.revertedWithCustomError(libsTest, 'InvalidAddressString'); + + invalidAddressString = '1x1234567890123456789012345678901234567890'; + await expect( + libsTest.stringToAddress(invalidAddressString), + ).to.be.revertedWithCustomError(libsTest, 'InvalidAddressString'); + + invalidAddressString = '001234567890123456789012345678901234567890'; + await expect( + libsTest.stringToAddress(invalidAddressString), + ).to.be.revertedWithCustomError(libsTest, 'InvalidAddressString'); + + invalidAddressString = '0x12345678901234567890123456789012345678g9'; + await expect( + libsTest.stringToAddress(invalidAddressString), + ).to.be.revertedWithCustomError(libsTest, 'InvalidAddressString'); + }); + it('should convert string to bytes and back', async () => { const string = 'big test string'; const bytes = await libsTest.stringToBytes32(string); @@ -46,4 +78,31 @@ describe('LibsTest', () => { ); expect(await libsTest.bytes32ToString(bytes)).to.equal(string); }); + + it('should revert on invalid string to bytes conversion', async () => { + let invalidString = ''; + await expect( + libsTest.stringToBytes32(invalidString), + ).to.be.revertedWithCustomError(libsTest, 'InvalidStringLength'); + + invalidString = 'This is a string that is too long to convert to bytes32'; + await expect( + libsTest.stringToBytes32(invalidString), + ).to.be.revertedWithCustomError(libsTest, 'InvalidStringLength'); + }); + + it('should revert if safe native transfer fails', async () => { + const value = 10; + + await ownerWallet + .sendTransaction({ + to: libsTest.address, + value, + }) + .then((tx) => tx.wait()); + + await expect( + libsTest.nativeTransfer(noFallBack.address, value), + ).to.be.revertedWithCustomError(libsTest, 'NativeTransferFailed'); + }); });