Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: linting warnings and initializer logic #46

Merged
merged 1 commit into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .solhint-src.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"extends": "solhint:recommended",
"rules": {
"compiler-version": ["error", "0.8.24"]
"compiler-version": ["error", "0.8.24"],
"func-visibility": ["error", { "ignoreConstructors": true }]
}
}
3 changes: 2 additions & 1 deletion .solhint-test.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"compiler-version": ["error", "0.8.24"],
"no-unused-vars": "off",
"no-console": "off",
"func-name-mixedcase": "off"
"func-name-mixedcase": "off",
"func-visibility": ["error", { "ignoreConstructors": true }]
}
}
2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ fs_permissions = [{ access = "read-write", path = "./gas"},
{ access = "read", path = "./test/fixtures"}]
src = 'src'
out = 'out'
libs = ['lib']
libs = ['lib', 'node_modules']
solc_version = "0.8.24"
test = 'test'
via_ir = true
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"scripts": {
"clean": "rm -rf cache artifacts typechain-types",
"build": "forge build",
"lint": "solhint -w 103 -c .solhint-src.json './src/**/*.sol' && solhint -w 241 -c .solhint-test.json './test/**/*.sol' && solhint -w 0 -c .solhint-script.json './script/**/*.sol'",
"lint": "solhint -w 0 -c .solhint-src.json './src/**/*.sol' && solhint -w 0 -c .solhint-test.json './test/**/*.sol' && solhint -w 0 -c .solhint-script.json './script/**/*.sol'",
"test": "forge test -vv",
"gasreport": "forge test --gas-report > gas/reports/gas-report.txt",
"coverage": "forge coverage --ir-minimum",
Expand Down
2 changes: 1 addition & 1 deletion script/002_DeployUpgradableMSCAFactory.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ contract DeployUpgradableMSCAFactoryScript is Script {
factory = UpgradableMSCAFactory(EXPECTED_FACTORY_ADDRESS);
console.log("Found existing factory at expected address: %s", address(factory));
}
console.log("Account implementation address: %s", address(factory.accountImplementation()));
console.log("Account implementation address: %s", address(factory.ACCOUNT_IMPLEMENTATION()));
vm.stopBroadcast();
}
}
2 changes: 1 addition & 1 deletion script/003_DeploySingleOwnerMSCAFactory.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract DeploySingleOwnerMSCAFactoryScript is Script {
factory = SingleOwnerMSCAFactory(EXPECTED_FACTORY_ADDRESS);
console.log("Found existing single owner MSCA factory at expected address: %s", address(factory));
}
console.log("Account implementation address: %s", address(factory.accountImplementation()));
console.log("Account implementation address: %s", address(factory.ACCOUNT_IMPLEMENTATION()));
vm.stopBroadcast();
}
}
2 changes: 1 addition & 1 deletion script/005_DeployECDSAAccountFactory.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ contract DeployECDSAAccountFactoryScript is Script {
factory = ECDSAAccountFactory(EXPECTED_FACTORY_ADDRESS);
}
console.log("Factory address: %s", address(factory));
console.log("Account implementation address: %s", address(factory.accountImplementation()));
console.log("Account implementation address: %s", address(factory.ACCOUNT_IMPLEMENTATION()));
vm.stopBroadcast();
}
}
21 changes: 14 additions & 7 deletions src/account/CoreAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
pragma solidity 0.8.24;

import {DefaultCallbackHandler} from "../callback/DefaultCallbackHandler.sol";
import {InvalidLength, UnauthorizedCaller} from "../common/Errors.sol";
import {ExecutionUtils} from "../utils/ExecutionUtils.sol";
import {BaseAccount} from "@account-abstraction/contracts/core/BaseAccount.sol";
import {IEntryPoint} from "@account-abstraction/contracts/interfaces/IEntryPoint.sol";
Expand All @@ -44,7 +45,7 @@ abstract contract CoreAccount is
{
// bytes4(keccak256("isValidSignature(bytes32,bytes)")
bytes4 internal constant EIP1271_MAGIC_VALUE = 0x1626ba7e;
IEntryPoint public immutable entryPointAddress;
IEntryPoint public immutable ENTRY_POINT_ADDRESS;

/**
* @dev This empty reserved space is put in place to allow future versions to add new
Expand All @@ -57,18 +58,21 @@ abstract contract CoreAccount is

function _checkOwner() internal view override {
// directly from EOA owner, or through the account itself (which gets redirected through execute())
require(msg.sender == owner() || msg.sender == address(this), "Caller is not the owner");
if (!(msg.sender == owner() || msg.sender == address(this))) {
revert UnauthorizedCaller();
}
}

/// @custom:oz-upgrades-unsafe-allow constructor
// for immutable values in implementations
constructor(IEntryPoint _newEntryPoint) {
entryPointAddress = _newEntryPoint;
ENTRY_POINT_ADDRESS = _newEntryPoint;
// lock the implementation contract so it can only be called from proxies
_disableInitializers();
}

// for mutable values in proxies
// solhint-disable-next-line func-name-mixedcase
function __CoreAccount_init(address _newOwner) internal onlyInitializing {
__Ownable_init();
transferOwnership(_newOwner);
Expand All @@ -77,7 +81,7 @@ abstract contract CoreAccount is

/// @inheritdoc BaseAccount
function entryPoint() public view virtual override returns (IEntryPoint) {
return entryPointAddress;
return ENTRY_POINT_ADDRESS;
}

/// @dev Please override this method
Expand Down Expand Up @@ -105,8 +109,9 @@ abstract contract CoreAccount is
whenNotPaused
{
_requireFromEntryPointOrOwner();
require(dest.length == func.length, "wrong array lengths");
require(dest.length == value.length, "wrong array lengths");
if (dest.length != func.length || dest.length != value.length) {
revert InvalidLength();
}
for (uint256 i = 0; i < dest.length; i++) {
ExecutionUtils.callAndRevert(dest[i], value[i], func[i]);
}
Expand All @@ -116,7 +121,9 @@ abstract contract CoreAccount is
* @dev Require the function call went through EntryPoint or owner.
*/
function _requireFromEntryPointOrOwner() internal view {
require(msg.sender == address(entryPoint()) || msg.sender == owner(), "account: not EntryPoint or Owner");
if (!(msg.sender == address(entryPoint()) || msg.sender == owner())) {
revert UnauthorizedCaller();
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/account/v1/ECDSAAccount.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ contract ECDSAAccount is CoreAccount, UUPSUpgradeable, BaseERC712CompliantAccoun
/// @inheritdoc UUPSUpgradeable
// The {_authorizeUpgrade} function must be overridden to include access restriction to the upgrade mechanism.
// Authorize the owner to upgrade the contract.
// solhint-disable-next-line no-empty-blocks
function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}

/// @custom:oz-upgrades-unsafe-allow constructor
Expand Down
14 changes: 7 additions & 7 deletions src/account/v1/factory/ECDSAAccountFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
pragma solidity 0.8.24;

import "../ECDSAAccount.sol";
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import "@openzeppelin/contracts/utils/Create2.sol";
import {ECDSAAccount, IEntryPoint} from "../ECDSAAccount.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {Create2} from "@openzeppelin/contracts/utils/Create2.sol";

/**
* @dev Account factory that creates the upgradeable account signed and verified by ECDSA.
Expand All @@ -29,10 +29,10 @@ contract ECDSAAccountFactory {
event AccountCreated(address indexed proxy, address indexed owner);

// logic implementation
ECDSAAccount public immutable accountImplementation;
ECDSAAccount public immutable ACCOUNT_IMPLEMENTATION;

constructor(IEntryPoint _entryPoint) {
accountImplementation = new ECDSAAccount(_entryPoint);
ACCOUNT_IMPLEMENTATION = new ECDSAAccount(_entryPoint);
}

/**
Expand Down Expand Up @@ -76,7 +76,7 @@ contract ECDSAAccountFactory {
account = ECDSAAccount(
payable(
new ERC1967Proxy{salt: _salt}(
address(accountImplementation), abi.encodeCall(ECDSAAccount.initialize, (_owner))
address(ACCOUNT_IMPLEMENTATION), abi.encodeCall(ECDSAAccount.initialize, (_owner))
)
)
);
Expand All @@ -92,7 +92,7 @@ contract ECDSAAccountFactory {
bytes32 code = keccak256(
abi.encodePacked(
type(ERC1967Proxy).creationCode,
abi.encode(address(accountImplementation), abi.encodeCall(ECDSAAccount.initialize, (_owner)))
abi.encode(address(ACCOUNT_IMPLEMENTATION), abi.encodeCall(ECDSAAccount.initialize, (_owner)))
)
);
// call computeAddress(salt, bytecodeHash, address(this))
Expand Down
4 changes: 3 additions & 1 deletion src/callback/DefaultCallbackHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,7 @@ contract DefaultCallbackHandler is IERC721Receiver, IERC1155Receiver, IERC777Rec
uint256 amount,
bytes calldata userData,
bytes calldata operatorData
) external pure override {}
) external pure override
// solhint-disable-next-line no-empty-blocks
{}
}
28 changes: 28 additions & 0 deletions src/common/Errors.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2024 Circle Internet Group, Inc. All rights reserved.

* SPDX-License-Identifier: GPL-3.0-or-later

* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.

* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.

* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
pragma solidity 0.8.24;

/**
* @notice Throws when the caller is unexpected.
*/
error UnauthorizedCaller();

error InvalidLength();

error Unsupported();
2 changes: 2 additions & 0 deletions src/libs/CastLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ library CastLib {
function toAddressArray(SetValue[] memory values) internal pure returns (address[] memory addresses) {
bytes32[] memory valuesBytes;

// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
valuesBytes := values
}
Expand All @@ -37,6 +38,7 @@ library CastLib {
valuesBytes[i] >>= 96;
}

// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
addresses := valuesBytes
}
Expand Down
1 change: 1 addition & 0 deletions src/libs/MessageHashUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ library MessageHashUtils {
*/
function toTypedDataHash(bytes32 domainSeparator, bytes32 structHash) internal pure returns (bytes32 digest) {
/// @solidity memory-safe-assembly
// solhint-disable-next-line no-inline-assembly
assembly {
let ptr := mload(0x40)
mstore(ptr, hex"1901")
Expand Down
9 changes: 0 additions & 9 deletions src/msca/6900/shared/common/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@
*/
pragma solidity 0.8.24;

/**
* @notice Throws when the caller is unexpected.
*/
error UnauthorizedCaller();

/**
* @notice Throws when the selector is not found.
*/
Expand All @@ -49,10 +44,6 @@ error InvalidInitializationInput();

error Create2FailedDeployment();

error InvalidLength();

error Unsupported();

error NotImplemented(bytes4 selector, uint8 functionId);

error InvalidItem();
Expand Down
2 changes: 2 additions & 0 deletions src/msca/6900/shared/libs/AddressDLLLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ library AddressDLLLib {
results[count] = current;
current = dll.next[current];
}

// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
mstore(results, count)
}
Expand Down
1 change: 1 addition & 0 deletions src/msca/6900/shared/libs/Bytes4DLLLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ library Bytes4DLLLib {
results[count] = current;
current = dll.next[current];
}
// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
mstore(results, count)
}
Expand Down
Loading
Loading