From 812f80cd7724b67899ef3ba999864391bd122412 Mon Sep 17 00:00:00 2001 From: Akash Gianchandani Date: Mon, 18 Dec 2023 18:21:27 +0530 Subject: [PATCH 1/4] fix(eth-multisig-v4): add chain id of the coin in the network identifier WIN-1423 --- contracts/WalletSimple.sol | 17 +++++++++-------- contracts/coins/ArbethWalletSimple.sol | 16 ++++++++-------- contracts/coins/OpethWalletSimple.sol | 16 ++++++++-------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/contracts/WalletSimple.sol b/contracts/WalletSimple.sol index 1bc11fe..02e31fe 100644 --- a/contracts/WalletSimple.sol +++ b/contracts/WalletSimple.sol @@ -7,6 +7,7 @@ import './IForwarder.sol'; /** ERC721, ERC1155 imports */ import '@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol'; import '@openzeppelin/contracts/token/ERC1155/utils/ERC1155Receiver.sol'; +import '@openzeppelin/contracts/utils/Strings.sol'; /** * @@ -29,8 +30,8 @@ import '@openzeppelin/contracts/token/ERC1155/utils/ERC1155Receiver.sol'; * Unlike eth_sign, the message is not prefixed. * * The operationHash the result of keccak256(prefix, toAddress, value, data, expireTime). - * For ether transactions, `prefix` is "ETHER". - * For token transaction, `prefix` is "ERC20" and `data` is the tokenContractAddress. + * For ether transactions, `prefix` is chain id of the coin i.e. for eth mainnet it is "1". + * For token transaction, `prefix` is chain id + "-ERC20" i.e. for mainnet it is "1-ERC20" and `data` is the tokenContractAddress. * * */ @@ -93,8 +94,8 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver { * to allow this contract to be used by proxy with delegatecall, which will * not pick up on state variables */ - function getNetworkId() internal pure virtual returns (string memory) { - return 'ETHER'; + function getNetworkId() internal view virtual returns (string memory) { + return Strings.toString(block.chainid); } /** @@ -105,8 +106,8 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver { * to allow this contract to be used by proxy with delegatecall, which will * not pick up on state variables */ - function getTokenNetworkId() internal pure virtual returns (string memory) { - return 'ERC20'; + function getTokenNetworkId() internal view virtual returns (string memory) { + return string.concat(Strings.toString(block.chainid), '-ERC20'); } /** @@ -117,8 +118,8 @@ contract WalletSimple is IERC721Receiver, ERC1155Receiver { * to allow this contract to be used by proxy with delegatecall, which will * not pick up on state variables */ - function getBatchNetworkId() internal pure virtual returns (string memory) { - return 'ETHER-Batch'; + function getBatchNetworkId() internal view virtual returns (string memory) { + return string.concat(Strings.toString(block.chainid), '-Batch'); } /** diff --git a/contracts/coins/ArbethWalletSimple.sol b/contracts/coins/ArbethWalletSimple.sol index 7970f64..0fe579c 100644 --- a/contracts/coins/ArbethWalletSimple.sol +++ b/contracts/coins/ArbethWalletSimple.sol @@ -25,8 +25,8 @@ import '../WalletSimple.sol'; * Unlike eth_sign, the message is not prefixed. * * The operationHash the result of keccak256(prefix, toAddress, value, data, expireTime). - * For ether transactions, `prefix` is "ARBETH". - * For token transaction, `prefix` is "ARBETH-ERC20" and `data` is the tokenContractAddress. + * For ether transactions, `prefix` is chain id of the coin i.e. for arbitrum mainnet it is "42161" + * For token transaction, `prefix` is "42161-ERC20" and `data` is the tokenContractAddress. * * */ @@ -35,23 +35,23 @@ contract ArbethWalletSimple is WalletSimple { * Get the network identifier that signers must sign over * This provides protection signatures being replayed on other chains */ - function getNetworkId() internal override pure returns (string memory) { - return 'ARBETH'; + function getNetworkId() internal override view returns (string memory) { + return Strings.toString(block.chainid); } /** * Get the network identifier that signers must sign over for token transfers * This provides protection signatures being replayed on other chains */ - function getTokenNetworkId() internal override pure returns (string memory) { - return 'ARBETH-ERC20'; + function getTokenNetworkId() internal override view returns (string memory) { + return string.concat(Strings.toString(block.chainid), '-ERC20'); } /** * Get the network identifier that signers must sign over for batch transfers * This provides protection signatures being replayed on other chains */ - function getBatchNetworkId() internal override pure returns (string memory) { - return 'ARBETH-Batch'; + function getBatchNetworkId() internal override view returns (string memory) { + return string.concat(Strings.toString(block.chainid), '-Batch'); } } diff --git a/contracts/coins/OpethWalletSimple.sol b/contracts/coins/OpethWalletSimple.sol index c8f8626..f1ed494 100644 --- a/contracts/coins/OpethWalletSimple.sol +++ b/contracts/coins/OpethWalletSimple.sol @@ -25,8 +25,8 @@ import '../WalletSimple.sol'; * Unlike eth_sign, the message is not prefixed. * * The operationHash the result of keccak256(prefix, toAddress, value, data, expireTime). - * For ether transactions, `prefix` is "OPETH". - * For token transaction, `prefix` is "OPETH-ERC20" and `data` is the tokenContractAddress. + * For ether transactions, `prefix` is chain id of the coin i.e. for optimism mainnet it is "10" + * For token transaction, `prefix` is "10-ERC20" and `data` is the tokenContractAddress. * * */ @@ -35,23 +35,23 @@ contract OpethWalletSimple is WalletSimple { * Get the network identifier that signers must sign over * This provides protection signatures being replayed on other chains */ - function getNetworkId() internal override pure returns (string memory) { - return 'OPETH'; + function getNetworkId() internal override view returns (string memory) { + return Strings.toString(block.chainid); } /** * Get the network identifier that signers must sign over for token transfers * This provides protection signatures being replayed on other chains */ - function getTokenNetworkId() internal override pure returns (string memory) { - return 'OPETH-ERC20'; + function getTokenNetworkId() internal override view returns (string memory) { + return string.concat(Strings.toString(block.chainid), '-ERC20'); } /** * Get the network identifier that signers must sign over for batch transfers * This provides protection signatures being replayed on other chains */ - function getBatchNetworkId() internal override pure returns (string memory) { - return 'OPETH-Batch'; + function getBatchNetworkId() internal override view returns (string memory) { + return string.concat(Strings.toString(block.chainid), '-Batch'); } } From 978f1c7c676a89d06b065d05c17a56958df289fd Mon Sep 17 00:00:00 2001 From: Akash Gianchandani Date: Mon, 18 Dec 2023 19:13:07 +0530 Subject: [PATCH 2/4] fix(eth-multisig-v4): add chain id of the coin in the network identifier WIN-1423 --- test/gas.js | 4 ++-- test/walletFactory.js | 2 +- test/walletSimple.js | 24 ++++++++++++------------ 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/test/gas.js b/test/gas.js index c837eba..e266ad8 100644 --- a/test/gas.js +++ b/test/gas.js @@ -91,7 +91,7 @@ describe(`Wallet Operations Gas Usage`, function () { }); const operationHash = helpers.getSha3ForConfirmationTx( - 'ETHER', + '17000', destinationAccount, amount, data, @@ -148,7 +148,7 @@ describe(`Wallet Operations Gas Usage`, function () { // Get the operation hash to be signed const operationHash = helpers.getSha3ForBatchTx( - 'ETHER-Batch', + '17000-Batch', recipients.map((recipient) => recipient.address.toLowerCase()), recipients.map((recipient) => recipient.amount), expireTime, diff --git a/test/walletFactory.js b/test/walletFactory.js index 43f91ca..3ed7a14 100644 --- a/test/walletFactory.js +++ b/test/walletFactory.js @@ -97,7 +97,7 @@ describe('WalletFactory', function () { // Get the operation hash to be signed const expireTime = Math.floor(new Date().getTime() / 1000) + 60; const operationHash = helpers.getSha3ForConfirmationTx( - 'ETHER', + '17000', accounts[3].toLowerCase(), amount, '0x', diff --git a/test/walletSimple.js b/test/walletSimple.js index b844b88..0793e39 100644 --- a/test/walletSimple.js +++ b/test/walletSimple.js @@ -49,9 +49,9 @@ const SAFE_MODE_ACTIVATE_EVENT = 'SafeModeActivated'; const coins = [ { name: 'Eth', - nativePrefix: 'ETHER', - nativeBatchPrefix: 'ETHER-Batch', - tokenPrefix: 'ERC20', + nativePrefix: '17000', + nativeBatchPrefix: '17000-Batch', + tokenPrefix: '17000-ERC20', WalletSimple: EthWalletSimple }, { @@ -77,23 +77,23 @@ const coins = [ }, { name: 'Polygon', - nativePrefix: 'POLYGON', - nativeBatchPrefix: 'POLYGON-Batch', - tokenPrefix: 'POLYGON-ERC20', + nativePrefix: '80001', + nativeBatchPrefix: '80001-Batch', + tokenPrefix: '80001-ERC20', WalletSimple: PolygonWalletSimple }, { name: 'Arbeth', - nativePrefix: 'ARBETH', - nativeBatchPrefix: 'ARBETH-Batch', - tokenPrefix: 'ARBETH-ERC20', + nativePrefix: '11155111', + nativeBatchPrefix: '11155111-Batch', + tokenPrefix: '11155111-ERC20', WalletSimple: ArbethWalletSimple }, { name: 'Opeth', - nativePrefix: 'OPETH', - nativeBatchPrefix: 'OPETH-Batch', - tokenPrefix: 'OPETH-ERC20', + nativePrefix: '11155420', + nativeBatchPrefix: '11155420-Batch', + tokenPrefix: '11155420-ERC20', WalletSimple: OpethWalletSimple } ]; From 0055ae2b2bbdd39117a9647a3c948f90eadaac8e Mon Sep 17 00:00:00 2001 From: Akash Gianchandani Date: Mon, 18 Dec 2023 19:34:46 +0530 Subject: [PATCH 3/4] fix(eth-multisig-v4): add chain id of the coin in the network identifier WIN-1423 --- test/gas.js | 2 +- test/walletFactory.js | 2 +- test/walletSimple.js | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/gas.js b/test/gas.js index e266ad8..ad2ef6a 100644 --- a/test/gas.js +++ b/test/gas.js @@ -91,7 +91,7 @@ describe(`Wallet Operations Gas Usage`, function () { }); const operationHash = helpers.getSha3ForConfirmationTx( - '17000', + '1', destinationAccount, amount, data, diff --git a/test/walletFactory.js b/test/walletFactory.js index 3ed7a14..a1f9741 100644 --- a/test/walletFactory.js +++ b/test/walletFactory.js @@ -97,7 +97,7 @@ describe('WalletFactory', function () { // Get the operation hash to be signed const expireTime = Math.floor(new Date().getTime() / 1000) + 60; const operationHash = helpers.getSha3ForConfirmationTx( - '17000', + '1', accounts[3].toLowerCase(), amount, '0x', diff --git a/test/walletSimple.js b/test/walletSimple.js index 0793e39..4190ba4 100644 --- a/test/walletSimple.js +++ b/test/walletSimple.js @@ -49,9 +49,9 @@ const SAFE_MODE_ACTIVATE_EVENT = 'SafeModeActivated'; const coins = [ { name: 'Eth', - nativePrefix: '17000', - nativeBatchPrefix: '17000-Batch', - tokenPrefix: '17000-ERC20', + nativePrefix: '1', + nativeBatchPrefix: '1-Batch', + tokenPrefix: '1-ERC20', WalletSimple: EthWalletSimple }, { From 0c3a5c645d487d592ff43dc23550a7332a20102d Mon Sep 17 00:00:00 2001 From: Akash Gianchandani Date: Mon, 18 Dec 2023 21:30:37 +0530 Subject: [PATCH 4/4] fix(eth-multisig-v4): add chain id of the coin in the network identifier WIN-1423 --- test/gas.js | 12 +++++++----- test/walletFactory.js | 3 ++- test/walletSimple.js | 25 +++++++++++++------------ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/test/gas.js b/test/gas.js index ad2ef6a..b08db2f 100644 --- a/test/gas.js +++ b/test/gas.js @@ -90,8 +90,9 @@ describe(`Wallet Operations Gas Usage`, function () { value: amount }); + // By default the chain id of hardhat network is 31337 const operationHash = helpers.getSha3ForConfirmationTx( - '1', + '31337', destinationAccount, amount, data, @@ -120,7 +121,7 @@ describe(`Wallet Operations Gas Usage`, function () { .eq(destinationEndBalance) .should.be.true(); - checkGasUsed(99555, transaction.receipt.gasUsed); + checkGasUsed(100585, transaction.receipt.gasUsed); }); const sendBatchHelper = async (batchSize) => { @@ -147,8 +148,9 @@ describe(`Wallet Operations Gas Usage`, function () { }); // Get the operation hash to be signed + // By default the chain id of hardhat network is 31337 const operationHash = helpers.getSha3ForBatchTx( - '17000-Batch', + '31337-Batch', recipients.map((recipient) => recipient.address.toLowerCase()), recipients.map((recipient) => recipient.amount), expireTime, @@ -178,8 +180,8 @@ describe(`Wallet Operations Gas Usage`, function () { it('WalletSimple send batch [ @skip-on-coverage ]', async function () { const gasUsageByBatchSize = [ - 101810, 113113, 124451, 135753, 147126, 158442, 169709, 181023, 192338, - 203641 + 103386, 114701, 126027, 137341, 148655, 159971, 171285, 182599, 193902, + 205205 ]; for (let batchSize = 1; batchSize <= 10; batchSize++) { diff --git a/test/walletFactory.js b/test/walletFactory.js index a1f9741..d553150 100644 --- a/test/walletFactory.js +++ b/test/walletFactory.js @@ -96,8 +96,9 @@ describe('WalletFactory', function () { // Get the operation hash to be signed const expireTime = Math.floor(new Date().getTime() / 1000) + 60; + // By default the chain id of hardhat network is 31337 const operationHash = helpers.getSha3ForConfirmationTx( - '1', + '31337', accounts[3].toLowerCase(), amount, '0x', diff --git a/test/walletSimple.js b/test/walletSimple.js index 4190ba4..c6380d8 100644 --- a/test/walletSimple.js +++ b/test/walletSimple.js @@ -46,12 +46,13 @@ const FORWARDER_DEPOSITED_EVENT = 'ForwarderDeposited'; const TRANSACTED_EVENT = 'Transacted'; const SAFE_MODE_ACTIVATE_EVENT = 'SafeModeActivated'; +// By default the chain id of hardhat network is 31337 const coins = [ { name: 'Eth', - nativePrefix: '1', - nativeBatchPrefix: '1-Batch', - tokenPrefix: '1-ERC20', + nativePrefix: '31337', + nativeBatchPrefix: '31337-Batch', + tokenPrefix: '31337-ERC20', WalletSimple: EthWalletSimple }, { @@ -77,23 +78,23 @@ const coins = [ }, { name: 'Polygon', - nativePrefix: '80001', - nativeBatchPrefix: '80001-Batch', - tokenPrefix: '80001-ERC20', + nativePrefix: 'POLYGON', + nativeBatchPrefix: 'POLYGON-Batch', + tokenPrefix: 'POLYGON-ERC20', WalletSimple: PolygonWalletSimple }, { name: 'Arbeth', - nativePrefix: '11155111', - nativeBatchPrefix: '11155111-Batch', - tokenPrefix: '11155111-ERC20', + nativePrefix: '31337', + nativeBatchPrefix: '31337-Batch', + tokenPrefix: '31337-ERC20', WalletSimple: ArbethWalletSimple }, { name: 'Opeth', - nativePrefix: '11155420', - nativeBatchPrefix: '11155420-Batch', - tokenPrefix: '11155420-ERC20', + nativePrefix: '31337', + nativeBatchPrefix: '31337-Batch', + tokenPrefix: '31337-ERC20', WalletSimple: OpethWalletSimple } ];