chore(ethers): change to ethers and upgrade dependencies #55

jurajpiar · 2022-09-05T16:24:23Z

What

replaces Web3 with ethers.js
replaces Truffle with Hardhat
~~adapt contracts to 0.8.9 solidity version~~
replaces prettier solhint checks with hardhat solhint plugin

Why

to upgrade the library

lgtm-com · 2022-09-14T10:24:04Z

This pull request fixes 1 alert when merging b013360 into aa2c585 - view on LGTM.com

fixed alerts:

1 for Unused variable, import, function or class

lgtm-com · 2022-09-16T02:01:14Z

This pull request fixes 1 alert when merging 1f8a91a into aa2c585 - view on LGTM.com

fixed alerts:

1 for Unused variable, import, function or class

lgtm-com · 2022-09-26T15:23:23Z

This pull request fixes 1 alert when merging b3ca582 into aa2c585 - view on LGTM.com

fixed alerts:

1 for Unused variable, import, function or class

lgtm-com · 2022-09-28T18:01:30Z

This pull request fixes 1 alert when merging d65de2a into aa2c585 - view on LGTM.com

fixed alerts:

1 for Unused variable, import, function or class

lgtm-com · 2022-10-04T01:07:26Z

This pull request fixes 1 alert when merging dc471e9 into aa2c585 - view on LGTM.com

fixed alerts:

1 for Unused variable, import, function or class

lgtm-com · 2022-10-04T17:59:15Z

This pull request fixes 1 alert when merging 0b966f5 into aa2c585 - view on LGTM.com

fixed alerts:

1 for Unused variable, import, function or class

ironFe93

left a comment, please review

ironFe93 · 2022-10-04T19:48:41Z

scripts/deploy.ts

+  }
+};
+
+export const writeConfiig = (config: NetworkAddresses) => {


Suggested change

export const writeConfiig = (config: NetworkAddresses) => {

export const writeConfig = (config: NetworkAddresses) => {

ironFe93 · 2022-10-04T19:48:55Z

scripts/deploy.ts

+  const contractAddresses = await deployContracts();
+  console.table(contractAddresses);
+  const newConfig = generateJsonConfig(contractAddresses);
+  writeConfiig(newConfig);


Suggested change

writeConfiig(newConfig);

writeConfig(newConfig);

test/scripts/deploy.test.ts

jurajpiar · 2022-10-05T10:43:39Z

I invite you to rebase this on master now to resolve conflicts.

contracts/smartwallet/SmartWallet.sol

contracts/smartwallet/CustomSmartWallet.sol

contracts/Collector.sol

-    validShares(_partners)
-    {   
+        RevenuePartner[] memory partners,
+        address remainderAddress


contracts/Collector.sol

contracts/smartwallet/CustomSmartWallet.sol

-                hex"a9059cbb",
-                tokenRecipient,
-                tokenAmount));
+            (bool success, bytes memory ret) = tokenAddr.call{gas: tokenGas}(


contracts/smartwallet/SmartWallet.sol

            );
        }

-
-            //Why this require is not needed: in the case that the EVM implementation 
-            //sends gasleft() as req.gas  if gasleft() < req.gas (see EIP-1930),  which would end in the call reverting
-            //If the relayer made this on purpose in order to collect the payment, since all gasLeft()
-            //was sent to this call, then the next line would give an out of gas, and, as a consequence, will
-            //revert the whole transaction, and the payment will not happen
-            //But it could happen that the destination call makes a gasleft() check and decides to revert if it is
-            //not enough, in that case there might be enough gas to complete the relay and the token payment would be collected
-            //For that reason, the next require line must be left uncommented, to avoid malicious relayer attacks to destination contract
-            //methods that revert if the gasleft() is not enough to execute whatever logic they have.
-
-            require(gasleft() > req.gas,"Not enough gas left");
-            (success, ret) = req.to.call{gas: req.gas, value: req.value}(req.data);
-
-            //If any balance has been added then trasfer it to the owner EOA
-            uint256 balanceToTransfer = address(this).balance;
-            if ( balanceToTransfer > 0) {
-                //can't fail: req.from signed (off-chain) the request, so it must be an EOA...
-                payable(req.from).transfer(balanceToTransfer);
-            }
-
-    } 
-
-    function getChainID() private pure returns (uint256 id) {
+        //Why this require is not needed: in the case that the EVM implementation
+        //sends gasleft() as req.gas  if gasleft() < req.gas (see EIP-1930),  which would end in the call reverting
+        //If the relayer made this on purpose in order to collect the payment, since all gasLeft()
+        //was sent to this call, then the next line would give an out of gas, and, as a consequence, will
+        //revert the whole transaction, and the payment will not happen
+        //But it could happen that the destination call makes a gasleft() check and decides to revert if it is
+        //not enough, in that case there might be enough gas to complete the relay and the token payment would be collected
+        //For that reason, the next require line must be left uncommented, to avoid malicious relayer attacks to destination contract
+        //methods that revert if the gasleft() is not enough to execute whatever logic they have.
+
+        require(gasleft() > req.gas, "Not enough gas left");
+        (success, ret) = req.to.call{gas: req.gas, value: req.value}(req.data);
+
+        //If any balance has been added then trasfer it to the owner EOA
+        uint256 balanceToTransfer = address(this).balance;
+        if (balanceToTransfer > 0) {
+            //can't fail: req.from signed (off-chain) the request, so it must be an EOA...
+            payable(req.from).transfer(balanceToTransfer);
+        }
+    }


contracts/Collector.sol

-    override
-    onlyOwner
-    {
+    function transferOwnership(address _owner) external override onlyOwner {


contracts/Migrations.sol

@@ -4,17 +4,17 @@
 contract Migrations {
    address public owner;
    // solhint-disable-next-line var-name-mixedcase
-    uint public last_completed_migration;
+    uint256 public last_completed_migration;


contracts/Penalizer.sol

+        bytes memory dataToCheck1 = abi.encodePacked(
+            decodedTx1.data,
+            decodedTx1.gasLimit,
+            decodedTx1.to,
+            decodedTx1.value
+        );


contracts/Penalizer.sol

+    string public override versionPenalizer =
+        "2.0.1+enveloping.penalizer.ipenalizer";


contracts/TokenHandler.sol

+    function removeToken(address token, uint256 index) external onlyOwner {
+        require(token != address(0), "Token cannot be zero address");
+        require(tokens[token] == true, "Token is not accepted");
+        require(token == acceptedTokens[index], "Wrong token index");
+        delete tokens[token];
+        acceptedTokens[index] = acceptedTokens[acceptedTokens.length - 1];
+        acceptedTokens.pop();
+    }


contracts/TokenHandler.sol

+    function acceptToken(address token) external onlyOwner {
+        require(token != address(0), "Token cannot be zero address");
+        require(tokens[token] == false, "Token is already accepted");
+        tokens[token] = true;
+        acceptedTokens.push(token);
+    }


contracts/RelayHub.sol

-    {
+    function penalize(
+        address relayWorker,
+        address payable beneficiary


contracts/smartwallet/CustomSmartWallet.sol

-    {
-
+    function directExecute(
+        address to,


contracts/smartwallet/SmartWallet.sol

-    {
-
+    function directExecute(
+        address to,


contracts/Collector.sol

-    noBalanceToShare
-    {
+    function updateRemainderAddress(
+        address remainderAddress


contracts/smartwallet/SmartWallet.sol

            );
        }

-
-            //Why this require is not needed: in the case that the EVM implementation 
-            //sends gasleft() as req.gas  if gasleft() < req.gas (see EIP-1930),  which would end in the call reverting
-            //If the relayer made this on purpose in order to collect the payment, since all gasLeft()
-            //was sent to this call, then the next line would give an out of gas, and, as a consequence, will
-            //revert the whole transaction, and the payment will not happen
-            //But it could happen that the destination call makes a gasleft() check and decides to revert if it is
-            //not enough, in that case there might be enough gas to complete the relay and the token payment would be collected
-            //For that reason, the next require line must be left uncommented, to avoid malicious relayer attacks to destination contract
-            //methods that revert if the gasleft() is not enough to execute whatever logic they have.
-
-            require(gasleft() > req.gas,"Not enough gas left");
-            (success, ret) = req.to.call{gas: req.gas, value: req.value}(req.data);
-
-            //If any balance has been added then trasfer it to the owner EOA
-            uint256 balanceToTransfer = address(this).balance;
-            if ( balanceToTransfer > 0) {
-                //can't fail: req.from signed (off-chain) the request, so it must be an EOA...
-                payable(req.from).transfer(balanceToTransfer);
-            }
-
-    } 
-
-    function getChainID() private pure returns (uint256 id) {
+        //Why this require is not needed: in the case that the EVM implementation
+        //sends gasleft() as req.gas  if gasleft() < req.gas (see EIP-1930),  which would end in the call reverting
+        //If the relayer made this on purpose in order to collect the payment, since all gasLeft()
+        //was sent to this call, then the next line would give an out of gas, and, as a consequence, will
+        //revert the whole transaction, and the payment will not happen
+        //But it could happen that the destination call makes a gasleft() check and decides to revert if it is
+        //not enough, in that case there might be enough gas to complete the relay and the token payment would be collected
+        //For that reason, the next require line must be left uncommented, to avoid malicious relayer attacks to destination contract
+        //methods that revert if the gasleft() is not enough to execute whatever logic they have.
+
+        require(gasleft() > req.gas, "Not enough gas left");
+        (success, ret) = req.to.call{gas: req.gas, value: req.value}(req.data);
+
+        //If any balance has been added then trasfer it to the owner EOA
+        uint256 balanceToTransfer = address(this).balance;
+        if (balanceToTransfer > 0) {
+            //can't fail: req.from signed (off-chain) the request, so it must be an EOA...
+            payable(req.from).transfer(balanceToTransfer);
+        }
+    }


feat(upgrade): re-create project with hardhat & ethers removing truffle and web3 chore: re-adds contracts chore: adds dev rules chore(contracts): fixes solidity lint errors chore(test): re-adds tests chore(upgrade): adds deployment tools chore(upgrade): add smock for contract mocking chore(upgrade): removes sol tests chore(upgrade): fixe pre-commit and update readme chore(upgrade): add more eslint rules chore(deps): move typechain to deps chore(upgrade): unignore dist from npm i chore(ethers): return util token for manual testing 👿 PP-565: migrate the removeTokens script to use Hardhat and Ethers.js (#92) * refactor: migrate the removeTokens script to use Hardhat and Ethers.js * fix: fix the task in hardhat.config, update Readme and apply format Co-authored-by: Antonio Morrone <antonio@iovlabs.org> PP-566: migrate script deploy collector (#95) * feat: dummy commit, create empty deployCollector.ts file * refactor: add script and task to deploy a Collector using Hardhat and Ethers.js * style: apply format rules * test: add the tests, remove the positional arguments and simplify the output file * fix: apply PR suggestions Remove some scripts from the package.json Use console.table to print the collector output Rename the input config file parameter Co-authored-by: Antonio Morrone <antonio@iovlabs.org> PP-569: migrate change partners (#97) * chore: change package.json version * feat: migrate the change-partners script to use ether.js * fix: change the parseJsonFile function to return undefined * fix: apply PR suggestions * Revert "style: include automatically formatted solidity files" This reverts commit 76ee863. * fix: remove the script from package.json and update Readme.md * style: apply formatting rules PP-567 Migrate Withdraw Task (#93) * feat: new hardhat task * feat: create withdraw task * chore: update readme * feat: complete withdrawal task and tests * chore: linted * fix: no from parameter on withdraw * chore: change readme, description of task * feat: reorganize tasks and tests * fix: restore .prepare script * fix: typo on script * fix: restore file permissions * fix: prettier issues, no waffle json file * fix: no prepare task * chore: remove unneeded rules * fix: change Readme.md to fix the command instruction * fix: restore the collector:deploy sample config file Co-authored-by: Christos Otarola <christos@Christoss-MacBook-Pro.local> Co-authored-by: Antonio Morrone <antonio@iovlabs.org> PP-355/update-rif-relay-server (#96) * refactor(index): expose typing for relay-server * refactor: prettier fix: fix the collector and add the missing tests Fix the collector updateRemainderAddress Include the missing test for the stakeForAddress method fix: add config to use custom signers to run scripts (#106) feat: expiration mechanism (#105) * feat: expiration mechanism * fix: FWD to SW * feat: validate time on deploy Co-authored-by: Christos Otarola <christos@Christoss-MacBook-Pro.local> PP-602/RelayClient: _validateSmartWallet (#109) * refactor(SmartWallet): expose getOwner * refactor(index): expose PromiseOrValue * chore: github workflow PP-358: include the contracts in dist (#112) * fix: remove contracts from npmignore * chore: rename .prettierrc to prettier.json5 to be consistent among all the repos PP-357: Add a task to mint utility tokens (#113) * feat: add a task to mint utility tokens * fix: apply linting and format to tasks * refactor: expose RelayHubInterface * docs: readme Co-authored-by: Francisco Tobar <francisco.tobar@iovlabs.org>

* chore(ethers): change to ethers and updgrade dependencies feat(upgrade): re-create project with hardhat & ethers removing truffle and web3 chore: re-adds contracts chore: adds dev rules chore(contracts): fixes solidity lint errors chore(test): re-adds tests chore(upgrade): adds deployment tools chore(upgrade): add smock for contract mocking chore(upgrade): removes sol tests chore(upgrade): fixe pre-commit and update readme chore(upgrade): add more eslint rules chore(deps): move typechain to deps chore(upgrade): unignore dist from npm i chore(ethers): return util token for manual testing 👿 PP-565: migrate the removeTokens script to use Hardhat and Ethers.js (#92) * refactor: migrate the removeTokens script to use Hardhat and Ethers.js * fix: fix the task in hardhat.config, update Readme and apply format Co-authored-by: Antonio Morrone <antonio@iovlabs.org> PP-566: migrate script deploy collector (#95) * feat: dummy commit, create empty deployCollector.ts file * refactor: add script and task to deploy a Collector using Hardhat and Ethers.js * style: apply format rules * test: add the tests, remove the positional arguments and simplify the output file * fix: apply PR suggestions Remove some scripts from the package.json Use console.table to print the collector output Rename the input config file parameter Co-authored-by: Antonio Morrone <antonio@iovlabs.org> PP-569: migrate change partners (#97) * chore: change package.json version * feat: migrate the change-partners script to use ether.js * fix: change the parseJsonFile function to return undefined * fix: apply PR suggestions * Revert "style: include automatically formatted solidity files" This reverts commit 76ee863. * fix: remove the script from package.json and update Readme.md * style: apply formatting rules PP-567 Migrate Withdraw Task (#93) * feat: new hardhat task * feat: create withdraw task * chore: update readme * feat: complete withdrawal task and tests * chore: linted * fix: no from parameter on withdraw * chore: change readme, description of task * feat: reorganize tasks and tests * fix: restore .prepare script * fix: typo on script * fix: restore file permissions * fix: prettier issues, no waffle json file * fix: no prepare task * chore: remove unneeded rules * fix: change Readme.md to fix the command instruction * fix: restore the collector:deploy sample config file Co-authored-by: Christos Otarola <christos@Christoss-MacBook-Pro.local> Co-authored-by: Antonio Morrone <antonio@iovlabs.org> PP-355/update-rif-relay-server (#96) * refactor(index): expose typing for relay-server * refactor: prettier fix: fix the collector and add the missing tests Fix the collector updateRemainderAddress Include the missing test for the stakeForAddress method fix: add config to use custom signers to run scripts (#106) feat: expiration mechanism (#105) * feat: expiration mechanism * fix: FWD to SW * feat: validate time on deploy Co-authored-by: Christos Otarola <christos@Christoss-MacBook-Pro.local> PP-602/RelayClient: _validateSmartWallet (#109) * refactor(SmartWallet): expose getOwner * refactor(index): expose PromiseOrValue * chore: github workflow PP-358: include the contracts in dist (#112) * fix: remove contracts from npmignore * chore: rename .prettierrc to prettier.json5 to be consistent among all the repos PP-357: Add a task to mint utility tokens (#113) * feat: add a task to mint utility tokens * fix: apply linting and format to tasks * refactor: expose RelayHubInterface * docs: readme Co-authored-by: Francisco Tobar <francisco.tobar@iovlabs.org> * fix: apply format and lint rules --------- Co-authored-by: Francisco Tobar <francisco.tobar@iovlabs.org> Co-authored-by: Antonio Morrone <antonio@iovlabs.org>

jurajpiar force-pushed the feature/PP-278/PP-329/upgrade_contracts branch 2 times, most recently from cde87f0 to b013360 Compare September 14, 2022 10:15

jurajpiar changed the title ~~chore(upgrade): change to ethers and updgrade dependencies~~ chore(ethers): change to ethers and updgrade dependencies Sep 19, 2022

jurajpiar changed the title ~~chore(ethers): change to ethers and updgrade dependencies~~ chore(ethers): change to ethers and upgrade dependencies Sep 19, 2022

ironFe93 reviewed Oct 4, 2022

View reviewed changes

jurajpiar force-pushed the feature/PP-278/PP-329/upgrade_contracts branch 4 times, most recently from d3ac8a4 to 2c76164 Compare October 5, 2022 10:33

AndresQuijano force-pushed the feature/PP-278/PP-329/upgrade_contracts branch from 27f6402 to 4e596d3 Compare October 14, 2022 16:55

antomor force-pushed the feature/PP-278/PP-329/upgrade_contracts branch 2 times, most recently from daed972 to 9661766 Compare October 21, 2022 13:43

github-advanced-security bot found potential problems Oct 21, 2022

View reviewed changes

github-advanced-security bot found potential problems Nov 10, 2022

View reviewed changes

antomor force-pushed the feature/PP-278/PP-329/upgrade_contracts branch from 8a5f55c to 9c2696d Compare November 16, 2022 11:30

github-advanced-security bot found potential problems Nov 24, 2022

View reviewed changes

jurajpiar mentioned this pull request Nov 28, 2022

feat(ethers): adds custom signers config #103

Closed

antomor added the do not merge label Nov 30, 2022

antomor force-pushed the feature/PP-278/PP-329/upgrade_contracts branch from 608bb6f to 867c2d4 Compare November 30, 2022 15:29

github-advanced-security bot found potential problems Dec 19, 2022

View reviewed changes

antomor changed the base branch from main to develop January 25, 2023 13:28

antomor changed the base branch from develop to main January 25, 2023 13:28

antomor changed the base branch from main to develop January 25, 2023 13:29

antomor force-pushed the feature/PP-278/PP-329/upgrade_contracts branch from 8fac133 to 4e2a5bd Compare January 26, 2023 13:03

antomor force-pushed the feature/PP-278/PP-329/upgrade_contracts branch from 4e2a5bd to 7ed4b21 Compare January 26, 2023 15:08

fix: apply format and lint rules

8a8caa3

antomor marked this pull request as ready for review January 26, 2023 15:20

antomor merged commit a805476 into develop Jan 27, 2023

antomor deleted the feature/PP-278/PP-329/upgrade_contracts branch February 6, 2023 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(ethers): change to ethers and upgrade dependencies #55

chore(ethers): change to ethers and upgrade dependencies #55

jurajpiar commented Sep 5, 2022 •

edited by antomor

lgtm-com bot commented Sep 14, 2022

lgtm-com bot commented Sep 16, 2022

lgtm-com bot commented Sep 26, 2022

lgtm-com bot commented Sep 28, 2022

lgtm-com bot commented Oct 4, 2022

lgtm-com bot commented Oct 4, 2022

ironFe93 left a comment

ironFe93 Oct 4, 2022

ironFe93 Oct 4, 2022

jurajpiar Oct 4, 2022

jurajpiar commented Oct 5, 2022

	export const writeConfiig = (config: NetworkAddresses) => {
	export const writeConfig = (config: NetworkAddresses) => {

		string public override versionPenalizer =
		"2.0.1+enveloping.penalizer.ipenalizer";

chore(ethers): change to ethers and upgrade dependencies #55

chore(ethers): change to ethers and upgrade dependencies #55

Conversation

jurajpiar commented Sep 5, 2022 • edited by antomor

What

Why

lgtm-com bot commented Sep 14, 2022

lgtm-com bot commented Sep 16, 2022

lgtm-com bot commented Sep 26, 2022

lgtm-com bot commented Sep 28, 2022

lgtm-com bot commented Oct 4, 2022

lgtm-com bot commented Oct 4, 2022

ironFe93 left a comment

Choose a reason for hiding this comment

ironFe93 Oct 4, 2022

Choose a reason for hiding this comment

ironFe93 Oct 4, 2022

Choose a reason for hiding this comment

jurajpiar Oct 4, 2022

Choose a reason for hiding this comment

jurajpiar commented Oct 5, 2022

jurajpiar commented Sep 5, 2022 •

edited by antomor