diff --git a/contracts/utils/CosmosIFTSendCallConstructor.sol b/contracts/utils/CosmosIFTSendCallConstructor.sol index 5740d372d..81f62b21b 100644 --- a/contracts/utils/CosmosIFTSendCallConstructor.sol +++ b/contracts/utils/CosmosIFTSendCallConstructor.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.28; +// solhint-disable gas-strict-inequalities,code-complexity + import { IIFTSendCallConstructor } from "../interfaces/IIFTSendCallConstructor.sol"; import { Strings } from "@openzeppelin-contracts/utils/Strings.sol"; @@ -11,6 +13,10 @@ import { IERC165 } from "@openzeppelin-contracts/utils/introspection/IERC165.sol /// @notice Constructs ICS27-GMP call data for minting IFT tokens on Cosmos SDK-based counterparty chains /// @dev This implementation encodes mint calls in protojson format for Cosmos SDK IFT module contract CosmosIFTSendCallConstructor is IIFTSendCallConstructor, ERC165 { + /// @notice Error thrown when the receiver address is invalid + /// @param receiver The invalid receiver string + error CosmosIFTInvalidReceiver(string receiver); + /// @notice The type URL for the MsgIFTMint message in the TokenFactory module // natlint-disable-next-line MissingInheritdoc string public bridgeReceiveTypeUrl; @@ -37,6 +43,10 @@ contract CosmosIFTSendCallConstructor is IIFTSendCallConstructor, ERC165 { /// @inheritdoc IIFTSendCallConstructor /// @dev Encodes the mint call as protojson CosmosTx for Cosmos SDK ICS27-GMP module function constructMintCall(string calldata receiver, uint256 amount) external view returns (bytes memory) { + if (!_validateReceiver(receiver)) { + revert CosmosIFTInvalidReceiver(receiver); + } + return abi.encodePacked( "{\"messages\":[{\"@type\":\"", bridgeReceiveTypeUrl, @@ -56,4 +66,62 @@ contract CosmosIFTSendCallConstructor is IIFTSendCallConstructor, ERC165 { function supportsInterface(bytes4 interfaceId) public view virtual override(ERC165, IERC165) returns (bool) { return interfaceId == type(IIFTSendCallConstructor).interfaceId || super.supportsInterface(interfaceId); } + + /// @notice Validates the receiver string to ensure it is either a valid Ethereum address or a valid bech32 address + /// with an appropriate HRP + /// @param receiver The receiver string to validate + /// @return True if the receiver is valid, false otherwise + function _validateReceiver(string calldata receiver) internal pure returns (bool) { + if (bytes(receiver).length == 0) { + return false; + } + + // We allow eth addresses as receivers due to cosmos/evm + // NOTE: We ignore the return value of tryParseAddress because + // we only want to check if it is a valid address format + // slither-disable-next-line unused-return + (bool isAddress,) = Strings.tryParseAddress(receiver); + if (isAddress) { + return true; + } + + // We also allow bech32 addresses for cosmos chains + // We will only allow HRPs which consist of [a-z0-9] and do not contain "1". If the HRP contains "1", then we + // will reject the address. All known Cosmos SDK chains have HRPs that do not contain "1", so this is a + // reasonable heuristic. + // We only allow lowercase alphanumeric characters excluding "1", "b", "i", and "o" as per bech32 specification, + // but we do not enforce the full bech32 checksum as it is not critical for our use case. + bool isHrp = true; + for (uint256 i = 0; i < bytes(receiver).length; ++i) { + uint256 c = uint256(uint8(bytes(receiver)[i])); + if (isHrp) { + if (c == 0x31) { + // "1" + isHrp = false; + continue; + } + if ((c >= 0x61 && c <= 0x7A) || (c >= 0x30 && c <= 0x39)) { + // a-z, 0-9 + continue; + } + return false; + } else { + if ((c == 0x31) || (c == 0x62) || (c == 0x69) || (c == 0x6F)) { + // "1", "b", "i", "o" + return false; + } + if ((c >= 0x61 && c <= 0x7A) || (c >= 0x30 && c <= 0x39)) { + // a-z, 0-9 + continue; + } + return false; + } + } + + if (isHrp) { + return false; // must contain "1" separator + } + + return true; + } } diff --git a/e2e/interchaintestv8/cosmos_ethereum_ift_test.go b/e2e/interchaintestv8/cosmos_ethereum_ift_test.go index 5f893ba43..54ed90e27 100644 --- a/e2e/interchaintestv8/cosmos_ethereum_ift_test.go +++ b/e2e/interchaintestv8/cosmos_ethereum_ift_test.go @@ -878,7 +878,7 @@ func (s *CosmosEthereumIFTTestSuite) Test_IFTTransfer_FailedReceiveOnCosmos() { tc := s.setupIFTInfrastructure(ctx, s.prover, s.proofType) ethUserAddr := crypto.PubkeyToAddress(s.ethUser.PublicKey) - invalidCosmosAddr := "invalid-cosmos-address" + invalidCosmosAddr := "wf1aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" s.Require().True(s.Run("Mint tokens on Ethereum", func() { iftContract, err := evmift.NewContract(tc.ethIFTAddress, eth.RPCClient) diff --git a/e2e/interchaintestv8/e2esuite/setup_ethereum.go b/e2e/interchaintestv8/e2esuite/setup_ethereum.go index be79eadd2..2628e63f7 100644 --- a/e2e/interchaintestv8/e2esuite/setup_ethereum.go +++ b/e2e/interchaintestv8/e2esuite/setup_ethereum.go @@ -18,7 +18,8 @@ import ( ) const ( - baseAnvilChainID = 31337 + baseAnvilChainID = 31337 + anvilBlockGasLimit = "50000000" ) func (s *TestSuite) buildChainSpecs(cfg setupConfig) []*interchaintest.ChainSpec { @@ -33,9 +34,9 @@ func (s *TestSuite) buildChainSpecs(cfg setupConfig) []*interchaintest.ChainSpec // Single anvil: Cosmos chains + one Anvil specs := chainconfig.DefaultChainSpecs - specs = append(specs, &interchaintest.ChainSpec{ - ChainConfig: icfoundry.DefaultEthereumAnvilChainConfig("ethereum"), - }) + chainConfig := icfoundry.DefaultEthereumAnvilChainConfig("ethereum") + chainConfig.AdditionalStartArgs = append(chainConfig.AdditionalStartArgs, "--gas-limit", anvilBlockGasLimit) + specs = append(specs, &interchaintest.ChainSpec{ChainConfig: chainConfig}) return specs } @@ -46,7 +47,7 @@ func (s *TestSuite) buildAnvilSpecs(count int) []*interchaintest.ChainSpec { name := fmt.Sprintf("ethereum-%d", i) chainConfig := icfoundry.DefaultEthereumAnvilChainConfig(name) chainConfig.ChainID = chainID - chainConfig.AdditionalStartArgs = append(chainConfig.AdditionalStartArgs, "--chain-id", chainID) + chainConfig.AdditionalStartArgs = append(chainConfig.AdditionalStartArgs, "--chain-id", chainID, "--gas-limit", anvilBlockGasLimit) specs = append(specs, &interchaintest.ChainSpec{ChainConfig: chainConfig}) } return specs diff --git a/test/solidity-ibc/IFTTest.t.sol b/test/solidity-ibc/IFTTest.t.sol index a67893ffa..2ac1fb659 100644 --- a/test/solidity-ibc/IFTTest.t.sol +++ b/test/solidity-ibc/IFTTest.t.sol @@ -854,69 +854,6 @@ contract IFTTest is Test { assertTrue(IERC165(address(ift)).supportsInterface(erc165Id)); } - struct IFTMintTestCase { - string name; - bool ownable; - address caller; - IICS27GMPMsgs.AccountIdentifier accountId; - address receiver; - uint256 amount; - bytes expectedRevert; - } - - struct OnAckPacketTestCase { - string name; - address caller; - bool success; - IIBCAppCallbacks.OnAcknowledgementPacketCallback callback; - bytes expectedRevert; - } - - struct OnTimeoutPacketTestCase { - string name; - address caller; - IIBCAppCallbacks.OnTimeoutPacketCallback callback; - bytes expectedRevert; - } - - struct IFTTransferTestCase { - string name; - address caller; - bool ownable; - string clientId; - string receiver; - uint256 amount; - uint64 timeoutTimestamp; - bytes expectedRevert; - } - - struct RemoveIFTBridgeTestCase { - string name; - address caller; - bool ownable; - string clientId; - bytes expectedRevert; - } - - struct RegisterIFTBridgeTestCase { - string name; - address caller; - bool ownable; - string clientId; - address iftSendCallConstructor; - string counterpartyIFT; - bytes expectedRevert; - } - - struct UpgradeTestCase { - string name; - address caller; - bool ownable; - bytes expectedRevert; - } - - // CosmosIFTSendCallConstructor Tests - function test_cosmosCallConstructor_constructor() public { string memory typeUrl = "/wfchain.ift.MsgIFTMint"; string memory tokenDenom = "uatom"; @@ -929,45 +866,154 @@ contract IFTTest is Test { assertEq(constructor_.icaAddress(), ica); } - function test_cosmosCallConstructor_constructMintCall() public { - string memory typeUrl = "/wfchain.ift.MsgIFTMint"; - string memory tokenDenom = "testift"; - string memory ica = "wf1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq5a9p63"; - - CosmosIFTSendCallConstructor constructor_ = new CosmosIFTSendCallConstructor(typeUrl, tokenDenom, ica); - - string memory receiver = "wf1receiver123"; - uint256 amount = 1_000_000; - - bytes memory callData = constructor_.constructMintCall(receiver, amount); + function fixtureCosmosMintCallTC() public returns (CosmosMintCallTestCase[] memory) { + CosmosMintCallTestCase[] memory testCases = new CosmosMintCallTestCase[](19); - bytes memory expected = abi.encodePacked( - "{\"messages\":[{\"@type\":\"", - typeUrl, - "\",\"signer\":\"", - ica, - "\",\"denom\":\"", - tokenDenom, - "\",\"receiver\":\"", - receiver, - "\",\"amount\":\"", - Strings.toString(amount), - "\"}]}" - ); + testCases[0] = CosmosMintCallTestCase({ + name: "success: bech32 receiver", receiver: "wf1address234", amount: 1_000_000, expectedRevert: "" + }); + testCases[1] = CosmosMintCallTestCase({ + name: "success: hex address receiver", + receiver: Strings.toHexString(makeAddr("receiver")), + amount: 2, + expectedRevert: "" + }); + testCases[2] = CosmosMintCallTestCase({ + name: "success: cosmos16...", + receiver: "cosmos16a87dpmkutsfx7cddxt749urv62nkcajmr8nw2", + amount: 3, + expectedRevert: "" + }); + testCases[3] = CosmosMintCallTestCase({ + name: "success: cosmos1xmx...", + receiver: "cosmos1xmxsh7td3ga53afs2qpzskq4dl4g9a4x3uck7j", + amount: 4, + expectedRevert: "" + }); + testCases[4] = CosmosMintCallTestCase({ + name: "success: cosmos1t5u...", + receiver: "cosmos1t5u0jfg3ljsjrh2m9e47d4ny2hea7eehxrzdgd", + amount: 5, + expectedRevert: "" + }); + testCases[5] = CosmosMintCallTestCase({ + name: "success: cosmos1hs6...", + receiver: "cosmos1hs6sl3u2lmrwr58khexm4v9yjk333c7ts0rcuz", + amount: 6, + expectedRevert: "" + }); + testCases[6] = CosmosMintCallTestCase({ + name: "success: osmo10c3...", + receiver: "osmo10c3r27nekd9w7s3swwg3t9sf5hzrvqjftf2djk", + amount: 7, + expectedRevert: "" + }); + testCases[7] = CosmosMintCallTestCase({ + name: "success: osmo14dy...", + receiver: "osmo14dy272jhqz3gqduujt0gngqm76sg3e0fqhvzxa", + amount: 8, + expectedRevert: "" + }); + testCases[8] = CosmosMintCallTestCase({ + name: "success: osmo1hd97...", + receiver: "osmo1hd97pvymu9xx63xg3y4qn2sy4cfywtzv0527jg", + amount: 9, + expectedRevert: "" + }); + testCases[9] = CosmosMintCallTestCase({ + name: "success: osmo1txtz...", + receiver: "osmo1txtzjqtmgwjhgkdvwtdgfwhsda77ewhxtsclce", + amount: 10, + expectedRevert: "" + }); + testCases[10] = CosmosMintCallTestCase({ + name: "revert: empty receiver", + receiver: "", + amount: 10, + expectedRevert: abi.encodeWithSelector(CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "") + }); + testCases[11] = CosmosMintCallTestCase({ + name: "revert: missing separator", + receiver: "wf2345a", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf2345a" + ) + }); + testCases[12] = CosmosMintCallTestCase({ + name: "revert: invalid hrp capital", + receiver: "WF12345a", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "WF12345a" + ) + }); + testCases[13] = CosmosMintCallTestCase({ + name: "revert: invalid data char 1", + receiver: "wf1rece1ver", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf1rece1ver" + ) + }); + testCases[14] = CosmosMintCallTestCase({ + name: "revert: invalid data char i", + receiver: "wf1receiver", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf1receiver" + ) + }); + testCases[15] = CosmosMintCallTestCase({ + name: "revert: invalid data char o", + receiver: "wf1receiver", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf1receiver" + ) + }); + testCases[16] = CosmosMintCallTestCase({ + name: "revert: invalid data char b", + receiver: "wf1recebr", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf1recebr" + ) + }); + testCases[17] = CosmosMintCallTestCase({ + name: "revert: invalid data char {", + receiver: "wf1address{", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf1address{" + ) + }); + testCases[18] = CosmosMintCallTestCase({ + name: "revert: invalid data char \"", + receiver: "wf1address\"", + amount: 10, + expectedRevert: abi.encodeWithSelector( + CosmosIFTSendCallConstructor.CosmosIFTInvalidReceiver.selector, "wf1address\"" + ) + }); - assertEq(callData, expected); + return testCases; } - function testFuzz_cosmosCallConstructor_constructMintCall(uint256 amount) public { - string memory typeUrl = "/custom.module.MsgMint"; - string memory tokenDenom = "ibc/ABC123"; - string memory ica = "cosmos1ica456"; + function tableCosmosMintCallTest(CosmosMintCallTestCase memory cosmosMintCallTC) public { + string memory typeUrl = "/wfchain.ift.MsgIFTMint"; + string memory tokenDenom = "testift"; + string memory ica = "wf1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq5a9p63"; CosmosIFTSendCallConstructor constructor_ = new CosmosIFTSendCallConstructor(typeUrl, tokenDenom, ica); - string memory receiver = "cosmos1receiver789"; + if (cosmosMintCallTC.expectedRevert.length != 0) { + vm.expectRevert(cosmosMintCallTC.expectedRevert); + constructor_.constructMintCall(cosmosMintCallTC.receiver, cosmosMintCallTC.amount); + return; + } - bytes memory callData = constructor_.constructMintCall(receiver, amount); + bytes memory callData = constructor_.constructMintCall(cosmosMintCallTC.receiver, cosmosMintCallTC.amount); bytes memory expected = abi.encodePacked( "{\"messages\":[{\"@type\":\"", @@ -977,67 +1023,93 @@ contract IFTTest is Test { "\",\"denom\":\"", tokenDenom, "\",\"receiver\":\"", - receiver, + cosmosMintCallTC.receiver, "\",\"amount\":\"", - Strings.toString(amount), + Strings.toString(cosmosMintCallTC.amount), "\"}]}" ); assertEq(callData, expected); } - function test_cosmosCallConstructor_constructMintCall_zeroAmount() public { + function test_cosmosCallConstructor_supportsInterface() public { CosmosIFTSendCallConstructor constructor_ = new CosmosIFTSendCallConstructor("/wfchain.ift.MsgIFTMint", "testift", "wf1ica"); - bytes memory callData = constructor_.constructMintCall("wf1receiver", 0); - - assertTrue(callData.length > 0); - bytes memory expected = abi.encodePacked( - "{\"messages\":[{\"@type\":\"/wfchain.ift.MsgIFTMint\",\"signer\":\"wf1ica\",\"denom\":\"testift\",\"receiver\":\"wf1receiver\",\"amount\":\"0\"}]}" - ); - assertEq(callData, expected); - } - - function test_cosmosCallConstructor_constructMintCall_largeAmount() public { - CosmosIFTSendCallConstructor constructor_ = - new CosmosIFTSendCallConstructor("/wfchain.ift.MsgIFTMint", "testift", "wf1ica"); + assertTrue(constructor_.supportsInterface(type(IIFTSendCallConstructor).interfaceId)); - uint256 largeAmount = type(uint256).max; - bytes memory callData = constructor_.constructMintCall("wf1receiver", largeAmount); + bytes4 erc165Id = 0x01ffc9a7; + assertTrue(constructor_.supportsInterface(erc165Id)); - bytes memory expected = abi.encodePacked( - "{\"messages\":[{\"@type\":\"/wfchain.ift.MsgIFTMint\",\"signer\":\"wf1ica\",\"denom\":\"testift\",\"receiver\":\"wf1receiver\",\"amount\":\"", - Strings.toString(largeAmount), - "\"}]}" - ); - assertEq(callData, expected); + bytes4 randomId = 0xdeadbeef; + assertFalse(constructor_.supportsInterface(randomId)); } +} - function test_cosmosCallConstructor_emptyStrings() public { - CosmosIFTSendCallConstructor constructor_ = new CosmosIFTSendCallConstructor("", "", ""); +struct IFTMintTestCase { + string name; + bool ownable; + address caller; + IICS27GMPMsgs.AccountIdentifier accountId; + address receiver; + uint256 amount; + bytes expectedRevert; +} - assertEq(constructor_.bridgeReceiveTypeUrl(), ""); - assertEq(constructor_.denom(), ""); - assertEq(constructor_.icaAddress(), ""); +struct OnAckPacketTestCase { + string name; + address caller; + bool success; + IIBCAppCallbacks.OnAcknowledgementPacketCallback callback; + bytes expectedRevert; +} - bytes memory callData = constructor_.constructMintCall("receiver", 100); - bytes memory expected = abi.encodePacked( - "{\"messages\":[{\"@type\":\"\",\"signer\":\"\",\"denom\":\"\",\"receiver\":\"receiver\",\"amount\":\"100\"}]}" - ); - assertEq(callData, expected); - } +struct OnTimeoutPacketTestCase { + string name; + address caller; + IIBCAppCallbacks.OnTimeoutPacketCallback callback; + bytes expectedRevert; +} - function test_cosmosCallConstructor_supportsInterface() public { - CosmosIFTSendCallConstructor constructor_ = - new CosmosIFTSendCallConstructor("/wfchain.ift.MsgIFTMint", "testift", "wf1ica"); +struct IFTTransferTestCase { + string name; + address caller; + bool ownable; + string clientId; + string receiver; + uint256 amount; + uint64 timeoutTimestamp; + bytes expectedRevert; +} - assertTrue(constructor_.supportsInterface(type(IIFTSendCallConstructor).interfaceId)); +struct RemoveIFTBridgeTestCase { + string name; + address caller; + bool ownable; + string clientId; + bytes expectedRevert; +} - bytes4 erc165Id = 0x01ffc9a7; - assertTrue(constructor_.supportsInterface(erc165Id)); +struct RegisterIFTBridgeTestCase { + string name; + address caller; + bool ownable; + string clientId; + address iftSendCallConstructor; + string counterpartyIFT; + bytes expectedRevert; +} - bytes4 randomId = 0xdeadbeef; - assertFalse(constructor_.supportsInterface(randomId)); - } +struct UpgradeTestCase { + string name; + address caller; + bool ownable; + bytes expectedRevert; +} + +struct CosmosMintCallTestCase { + string name; + string receiver; + uint256 amount; + bytes expectedRevert; }