address audit feedback #120
Conversation
gianchandania
force-pushed
the
WIN-1733-address-audit-review-comments
branch
from
08a3542 to
48aa44f
Compare
gianchandania
force-pushed
the
WIN-1733-address-audit-review-comments
branch
from
b01ce39 to
165b4be
Compare
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@ethersproject/[email protected], npm/@ethersproject/[email protected], npm/@ethersproject/[email protected], npm/@ethersproject/[email protected], npm/@openzeppelin/[email protected], npm/[email protected] |
| @@ -71,12 +71,12 @@ contract Batcher { | |||
| // Revert everything if any transfer fails | |||
| for (uint8 i = 0; i < recipients.length; i++) { | |||
| require(recipients[i] != address(0), 'Invalid recipient address'); | |||
| emit BatchTransfer(msg.sender, recipients[i], values[i]); | |||
There was a problem hiding this comment.
I'm not sure if we even store a state in this function
| * Gets called when a transaction is received with data that does not match any other method | ||
| */ | ||
| fallback() external payable {} | ||
|
|
There was a problem hiding this comment.
I don't think this will impact erc20 or any other token transfers however I feel its a good safety net
gianchandania
force-pushed
the
WIN-1733-address-audit-review-comments
branch
from
2f46262 to
1779f56
Compare
contracts/WalletSimple.sol
Outdated
| for (uint256 i = 0; i < recipients.length; i++) { | ||
| require(recipients[i] != address(0), 'Invalid recipient address'); |
There was a problem hiding this comment.
Do we need this check added here, address zero might help you in confirming that your contract is handling uninitialised data correctly, But in this case since we are already checking recipients.length and only passing in if the value is there, this check seems redundant
There was a problem hiding this comment.
Removed the zero address validations
gianchandania
force-pushed
the
WIN-1733-address-audit-review-comments
branch
from
1779f56 to
12af091
Compare
gianchandania
force-pushed
the
WIN-1733-address-audit-review-comments
branch
from
12af091 to
705c17d
Compare
gianchandania
force-pushed
the
WIN-1733-address-audit-review-comments
branch
from
705c17d to
848a4cd
Compare
Commit 1: fix BGB-03
Commit 2: fix RSC-02
Commit 3: fix BGB-02
Commit 4: fix GLOBAL-01 In this commit ERC1155Receiver contract was removed because it got depreciated in openzeppelin/contracts version 5.0.0. Instead ERC1155Holder contract is used for the same purpose. The visibility of onERC1155Received and onERC1155BatchReceived method is changed to public because ERC1155Holder contract has these methods with public visibility
Commit 5: fix WSG-03
Commit 6: fix BGB-01
Commit 7: fix WSG-02
Commit 8: fix WSG-01
Commit 9: fix BAT-02. This change reverts the transaction if the sent out amount is not equal to the received eth amount. Some tests which are not relevant anymore are removed
Commit 10: fix BAC-01. This change removes test to transfer ownership to address(0) as it is not relevant, because in the current flow ownership needs to be accepted by the new owner.
Commit 11: fix WSG-06. This change doesn't change abi.encodePacked in the sendMultiSigToken as it has not been highlighted as an issue.
Commit 12: update optimizer runs Update optimizer runs in hardhat config
WIN-1733