Skip to content

Commit 7cf60b1

Browse files
authored
Audit fixes: account and evolving-nft (#487)
* Fix: [M-1] EntryPoint contract can change * Fix: [M-2] Use of forbidden TIMESTAMP op-code * [M-3] Scores using multiplicative rules for ERC20s can be inflated * Fix [M-4] Shared metadata will get out of order when deleting metadata * uncomment tests * Fix [L-1] Unable to upgrade receive() * Fix [L-2] Cannot remove upgradability without revoking all default admins * Fix: [Q-1] Duplicate code * Fix [L-3] isValidSignature should be upgradable * [Q-1]: Remove initialize from Account + Fix [L-4] Invalid accounts can register with Account factories * Fix [Q-2] Duplicate comment * Fix [Q-3] Constant MAX_BPS is not used * Fix [Q-4] Mistyped functions * Fix [Q-6] Inaccurate comment * Fix [Q-7] _setPlatformFeeType() is not used * Fix [Q-8] Spelling mistakes * Fix [Q-9] Duplicate Import * Fix (again) [Q-8] Spelling mistakes * Fix [G-1] platformFeeType can share a storage slot * Fix pt2 [M-3] Scores using multiplicative rules for ERC20s can be inflated * UPdate Fix for M-3 * Fix M-3: remove redundant code * Add missing EIP-1271 isValidSignature to Account * Add missing receive() to non-upgradeable Account * Fix EvolvingNFT tests * Remove receive fn from ManagedAccountFactory * Use internal function to access state * dynamic-contracts deps temp * Update dynamic-contracts deps * Fix test * Use extension role in managed account factory * Use dynamic-contracts v1.2.1
1 parent 261d352 commit 7cf60b1

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

51 files changed

+2084
-2027
lines changed

contracts/extension/PlatformFee.sol

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,12 @@ abstract contract PlatformFee is IPlatformFee {
1919
/// @dev The % of primary sales collected as platform fees.
2020
uint16 private platformFeeBps;
2121

22-
/// @dev The flat amount collected by the contract as fees on primary sales.
23-
uint256 private flatPlatformFee;
24-
2522
/// @dev Fee type variants: percentage fee and flat fee
2623
PlatformFeeType private platformFeeType;
2724

25+
/// @dev The flat amount collected by the contract as fees on primary sales.
26+
uint256 private flatPlatformFee;
27+
2828
/// @dev Returns the platform fee recipient and bps.
2929
function getPlatformFeeInfo() public view override returns (address, uint16) {
3030
return (platformFeeRecipient, uint16(platformFeeBps));
@@ -90,9 +90,7 @@ abstract contract PlatformFee is IPlatformFee {
9090
if (!_canSetPlatformFeeInfo()) {
9191
revert("Not authorized");
9292
}
93-
platformFeeType = _feeType;
94-
95-
emit PlatformFeeTypeUpdated(_feeType);
93+
_setupPlatformFeeType(_feeType);
9694
}
9795

9896
/// @dev Sets platform fee type.

contracts/extension/interface/IRulesEngine.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ interface IRulesEngine {
4848

4949
function getRulesEngineOverride() external view returns (address rulesEngineAddress);
5050

51-
function createRuleMulitiplicative(RuleTypeMultiplicative memory rule) external returns (bytes32 ruleId);
51+
function createRuleMultiplicative(RuleTypeMultiplicative memory rule) external returns (bytes32 ruleId);
5252

5353
function createRuleThreshold(RuleTypeThreshold memory rule) external returns (bytes32 ruleId);
5454

contracts/extension/upgradeable/RulesEngine.sol

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ pragma solidity ^0.8.11;
66
import "../interface/IRulesEngine.sol";
77

88
import "../../eip/interface/IERC20.sol";
9+
import "../../eip/interface/IERC20Metadata.sol";
910
import "../../eip/interface/IERC721.sol";
1011
import "../../eip/interface/IERC1155.sol";
1112

@@ -71,7 +72,7 @@ abstract contract RulesEngine is IRulesEngine {
7172
External functions
7273
//////////////////////////////////////////////////////////////*/
7374

74-
function createRuleMulitiplicative(RuleTypeMultiplicative memory rule) external returns (bytes32 ruleId) {
75+
function createRuleMultiplicative(RuleTypeMultiplicative memory rule) external returns (bytes32 ruleId) {
7576
require(_canSetRules(), "RulesEngine: cannot set rules");
7677

7778
ruleId = keccak256(
@@ -114,7 +115,9 @@ abstract contract RulesEngine is IRulesEngine {
114115
uint256 balance = 0;
115116

116117
if (_rule.tokenType == TokenType.ERC20) {
117-
balance = IERC20(_rule.token).balanceOf(_tokenOwner);
118+
// NOTE: We are rounding down the ERC20 balance to the nearest full unit.
119+
uint256 unit = 10**IERC20Metadata(_rule.token).decimals();
120+
balance = IERC20(_rule.token).balanceOf(_tokenOwner) / unit;
118121
} else if (_rule.tokenType == TokenType.ERC721) {
119122
balance = IERC721(_rule.token).balanceOf(_tokenOwner);
120123
} else if (_rule.tokenType == TokenType.ERC1155) {
@@ -143,7 +146,7 @@ abstract contract RulesEngine is IRulesEngine {
143146
}
144147

145148
function setRulesEngineOverride(address _rulesEngineAddress) external {
146-
require(_canOverrieRulesEngine(), "RulesEngine: cannot override rules engine");
149+
require(_canOverrideRulesEngine(), "RulesEngine: cannot override rules engine");
147150
_rulesEngineStorage().rulesEngineOverride = _rulesEngineAddress;
148151

149152
emit RulesEngineOverriden(_rulesEngineAddress);
@@ -155,5 +158,5 @@ abstract contract RulesEngine is IRulesEngine {
155158

156159
function _canSetRules() internal view virtual returns (bool);
157160

158-
function _canOverrieRulesEngine() internal view virtual returns (bool);
161+
function _canOverrideRulesEngine() internal view virtual returns (bool);
159162
}

contracts/external-deps/openzeppelin/ERC1155PresetUpgradeable.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ contract ERC1155PresetUpgradeable is
4545

4646
mapping(uint256 => uint256) private _totalSupply;
4747

48-
/// @dev Initiliazes the contract, like a constructor.
48+
/// @dev Initializes the contract, like a constructor.
4949
function __ERC1155Preset_init(address _deployer, string memory uri) internal onlyInitializing {
5050
// Initialize inherited contracts, most base-like -> most derived.
5151
__ERC1155_init(uri);

contracts/legacy-contracts/pre-builts/DropERC1155_V2.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ contract DropERC1155_V2 is
137137

138138
constructor() initializer {}
139139

140-
/// @dev Initiliazes the contract, like a constructor.
140+
/// @dev Initializes the contract, like a constructor.
141141
function initialize(
142142
address _defaultAdmin,
143143
string memory _name,

contracts/legacy-contracts/pre-builts/DropERC20_V2.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ contract DropERC20_V2 is
9393

9494
constructor() initializer {}
9595

96-
/// @dev Initiliazes the contract, like a constructor.
96+
/// @dev Initializes the contract, like a constructor.
9797
function initialize(
9898
address _defaultAdmin,
9999
string memory _name,

contracts/legacy-contracts/pre-builts/DropERC721_V3.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ contract DropERC721_V3 is
132132

133133
constructor() initializer {}
134134

135-
/// @dev Initiliazes the contract, like a constructor.
135+
/// @dev Initializes the contract, like a constructor.
136136
function initialize(
137137
address _defaultAdmin,
138138
string memory _name,

contracts/legacy-contracts/pre-builts/SignatureDrop_V4.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ contract SignatureDrop_V4 is
6363
Constructor + initializer logic
6464
//////////////////////////////////////////////////////////////*/
6565

66-
/// @dev Initiliazes the contract, like a constructor.
66+
/// @dev Initializes the contract, like a constructor.
6767
function initialize(
6868
address _defaultAdmin,
6969
string memory _name,

contracts/prebuilts/account/dynamic/DynamicAccount.sol

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ contract DynamicAccount is AccountCore, BaseRouter {
3333
/// @notice Initializes the smart contract wallet.
3434
function initialize(address _defaultAdmin, bytes calldata) public override initializer {
3535
__BaseRouter_init();
36+
AccountCoreStorage.data().firstAdmin = _defaultAdmin;
3637
_setAdmin(_defaultAdmin, true);
3738
}
3839

contracts/prebuilts/account/interface/IAccountFactory.sol

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,18 @@ interface IAccountFactory {
2323
function createAccount(address admin, bytes calldata _data) external returns (address account);
2424

2525
/// @notice Callback function for an Account to register its signers.
26-
function onSignerAdded(address signer) external;
26+
function onSignerAdded(
27+
address signer,
28+
address creatorAdmin,
29+
bytes memory data
30+
) external;
2731

2832
/// @notice Callback function for an Account to un-register its signers.
29-
function onSignerRemoved(address signer) external;
33+
function onSignerRemoved(
34+
address signer,
35+
address creatorAdmin,
36+
bytes memory data
37+
) external;
3038

3139
/*///////////////////////////////////////////////////////////////
3240
View Functions

contracts/prebuilts/account/managed/ManagedAccountFactory.sol

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ contract ManagedAccountFactory is BaseAccountFactory, ContractMetadata, Permissi
3232
{
3333
__BaseRouter_init();
3434
_setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
35-
}
3635

37-
receive() external payable {
38-
revert("Cannot accept ether.");
36+
bytes32 _extensionRole = keccak256("EXTENSION_ROLE");
37+
_setupRole(_extensionRole, msg.sender);
38+
_setRoleAdmin(_extensionRole, _extensionRole);
3939
}
4040

4141
/*///////////////////////////////////////////////////////////////
@@ -53,7 +53,7 @@ contract ManagedAccountFactory is BaseAccountFactory, ContractMetadata, Permissi
5353

5454
/// @dev Returns whether all relevant permission and other checks are met before any upgrade.
5555
function isAuthorizedCallToUpgrade() internal view virtual override returns (bool) {
56-
return hasRole(DEFAULT_ADMIN_ROLE, msg.sender);
56+
return hasRole(keccak256("EXTENSION_ROLE"), msg.sender);
5757
}
5858

5959
/// @dev Returns whether contract metadata can be set in the given execution context.

0 commit comments

Comments
 (0)