Skip to content

Commit

Permalink
Merge pull request #58 from KintoXYZ/useropmaxcost-setter
Browse files Browse the repository at this point in the history 
MixBytes: add user op max cost setter
  • Loading branch information
@fedealconada
fedealconada authored Jan 30, 2024
2 parents 21dfc41 + d5134ef commit 7239d0a
Show file tree
Hide file tree
Showing 9 changed files with 273 additions and 93 deletions.
71 changes: 44 additions & 27 deletions src/paymasters/SponsorPaymaster.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,17 @@ import "../interfaces/IKintoWallet.sol";
contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, ReentrancyGuard, ISponsorPaymaster {
using SafeERC20 for IERC20;

// ========== Events ============
event AppRegistrySet(address appRegistry, address _oldRegistry);

// calculated cost of the postOp
uint256 public constant COST_OF_POST = 200_000;
uint256 public constant MAX_COST_OF_VERIFICATION = 230_000;
uint256 public constant MAX_COST_OF_PREVERIFICATION = 110_000;
uint256 public constant MAX_COST_OF_USEROP = 0.03 ether;

uint256 public constant RATE_LIMIT_PERIOD = 1 minutes;
uint256 public constant RATE_LIMIT_THRESHOLD_TOTAL = 50;

uint256 public userOpMaxCost;
IKintoAppRegistry public override appRegistry;

mapping(address => uint256) public balances;
mapping(address => uint256) public contractSpent; // keeps track of total gas consumption by contract
mapping(address => uint256) public unlockBlock;
Expand All @@ -46,7 +45,10 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
// rate limit across apps: user => RateLimitData
mapping(address => ISponsorPaymaster.RateLimitData) public globalRateLimit;

IKintoAppRegistry public override appRegistry;
// ========== Events ============

event AppRegistrySet(address oldRegistry, address newRegistry);
event UserOpMaxCostSet(uint256 oldUserOpMaxCost, uint256 newUserOpMaxCost);

// ========== Constructor & Upgrades ============

Expand All @@ -62,6 +64,7 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
function initialize(address _owner) external virtual initializer {
__UUPSUpgradeable_init();
_transferOwnership(_owner);

// unlocks owner
unlockBlock[_owner] = block.number;
}
Expand All @@ -76,16 +79,6 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
(newImplementation);
}

/**
* @dev Set the app registry
* @param _appRegistry address of the app registry
*/
function setAppRegistry(address _appRegistry) external override onlyOwner {
require(_appRegistry != address(0) && _appRegistry != address(appRegistry), "SP: appRegistry cannot be 0");
emit AppRegistrySet(_appRegistry, address(appRegistry));
appRegistry = IKintoAppRegistry(_appRegistry);
}

// ========== Deposit Mgmt ============

/**
Expand Down Expand Up @@ -137,16 +130,15 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
entryPoint.withdrawTo(payable(target), amount);
}

/* =============== Viewers & validation ============= */
/* =============== Setters & Getters ============= */

/**
* Return the deposit info for the account
* @return amount - the amount of given token deposited to the Paymaster.
* @return _unlockBlock - the block height at which the deposit can be withdrawn.
*/
function depositInfo(address account) external view returns (uint256 amount, uint256 _unlockBlock) {
amount = balances[account];
_unlockBlock = unlockBlock[account];
return (balances[account], unlockBlock[account]);
}

/**
Expand All @@ -173,9 +165,30 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
}

/**
* Validate the request from the sender to fund it.
* The sender should have enough txs and gas left to be gasless.
* The contract developer funds the contract for its users and rate limits the app.
* @dev Set the app registry
* @param _newRegistry address of the app registry
*/
function setAppRegistry(address _newRegistry) external override onlyOwner {
require(_newRegistry != address(0) && _newRegistry != address(appRegistry), "SP: new registry cannot be 0");
emit AppRegistrySet(address(appRegistry), _newRegistry);
appRegistry = IKintoAppRegistry(_newRegistry);
}

/**
* @dev Set the max cost of a user operation
* @param _newUserOpMaxCost max cost of a user operation
*/
function setUserOpMaxCost(uint256 _newUserOpMaxCost) external onlyOwner {
emit UserOpMaxCostSet(userOpMaxCost, _newUserOpMaxCost);
userOpMaxCost = _newUserOpMaxCost;
}

/* =============== AA overrides ============= */

/**
* @notice Validates the request from the sender to fund it.
* @dev sender should have enough txs and gas left to be gasless.
* @dev contract developer funds the contract for its users and rate limits the app.
*/
function _validatePaymasterUserOp(UserOperation calldata userOp, bytes32 userOpHash, uint256 maxCost)
internal
Expand All @@ -196,7 +209,7 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
// use maxFeePerGas for conservative estimation of gas cost
uint256 gasPriceUserOp = userOp.maxFeePerGas;
uint256 ethMaxCost = (maxCost + COST_OF_POST * gasPriceUserOp);
require(ethMaxCost <= MAX_COST_OF_USEROP, "SP: gas too high for user op");
require(ethMaxCost <= userOpMaxCost, "SP: gas too high for user op");

address sponsor = appRegistry.getSponsor(_decodeCallData(userOp.callData));
require(unlockBlock[sponsor] == 0, "SP: deposit not locked");
Expand All @@ -205,7 +218,7 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
}

/**
* perform the post-operation to charge the account contract for the gas.
* @notice performs the post-operation to charge the account contract for the gas.
*/
function _postOp(PostOpMode, /* mode */ bytes calldata context, uint256 actualGasCost) internal override {
(address account, address userAccount, uint256 maxFeePerGas, uint256 maxPriorityFeePerGas) =
Expand Down Expand Up @@ -250,6 +263,8 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
_checkLimits(userAccount, account, ethCost);
}

/* =============== Internal methods ============= */

function _checkLimits(address sender, address targetAccount, uint256 ethMaxCost) internal view {
// global rate limit check
ISponsorPaymaster.RateLimitData memory globalData = globalRateLimit[sender];
Expand Down Expand Up @@ -277,10 +292,12 @@ contract SponsorPaymaster is Initializable, BasePaymaster, UUPSUpgradeable, Reen
);
}

// @notice extracts `target` contract from callData
// @dev the last op on a batch MUST always be a contract whose sponsor is the one we want to
// bear with the gas cost of all ops
// @dev this is very similar to KintoWallet._decodeCallData, consider unifying
/**
* @notice extracts `target` contract from callData
* @dev the last op on a batch MUST always be a contract whose sponsor is the one we want to
* bear with the gas cost of all ops
* @dev this is very similar to KintoWallet._decodeCallData, consider unifying
*/
function _decodeCallData(bytes calldata callData) private pure returns (address target) {
bytes4 selector = bytes4(callData[:4]); // extract the function selector from the callData

Expand Down
27 changes: 14 additions & 13 deletions src/wallet/KintoWallet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -376,27 +376,28 @@ contract KintoWallet is Initializable, BaseAccount, TokenCallbackHandler, IKinto
// @dev SINGLE_SIGNER policy expects the wallet to have only one owner though this is not enforced.
// Any "extra" owners won't be considered when validating the signature.
function _resetSigners(address[] calldata newSigners, uint8 _policy) internal {
require(newSigners.length > 0 && newSigners.length <= MAX_SIGNERS, "KW-rs: invalid array");
require(newSigners.length > 0 && newSigners.length <= MAX_SIGNERS, "KW-rs: signers exceed max limit");
require(newSigners[0] != address(0) && kintoID.isKYC(newSigners[0]), "KW-rs: KYC Required");
require(
newSigners.length == 1 || (newSigners.length == 2 && newSigners[0] != newSigners[1])
|| (
newSigners.length == 3 && (newSigners[0] != newSigners[1]) && (newSigners[1] != newSigners[2])
&& newSigners[0] != newSigners[2]
),
"duplicate owners"
);

// ensure no duplicate signers
for (uint256 i = 0; i < newSigners.length; i++) {
for (uint256 j = i + 1; j < newSigners.length; j++) {
require(newSigners[i] != newSigners[j], "KW-rs: duplicate signers");
}
}

// ensure all signers are valid
for (uint256 i = 0; i < newSigners.length; i++) {
require(newSigners[i] != address(0), "KW-rs: invalid signer address");
}

emit SignersChanged(owners, newSigners);
owners = newSigners;
emit SignersChanged(newSigners, owners);

// change policy if needed.
// change policy if needed
require(_policy == 1 || newSigners.length > 1, "KW-rs: invalid policy for single signer");
if (_policy != signerPolicy) {
setSignerPolicy(_policy);
} else {
require(_policy == 1 || newSigners.length > 1, "KW-rs: invalid policy");
}
}

Expand Down
24 changes: 14 additions & 10 deletions src/wallet/KintoWalletFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,18 +264,18 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
(newImplementation);
}

function _preventCreationBytecode(bytes calldata _bytes) internal view {
// Prevent direct deployment of KintoWallet contracts
function _preventWalletDeployment(bytes calldata _bytecode) internal view {
bytes memory walletInitCode = type(SafeBeaconProxy).creationCode;
if (_bytes.length > walletInitCode.length + 32) {
// Make sure this beacon cannot be called with creation code
if (_bytecode.length > walletInitCode.length + 32) {
uint256 offset = 12 + walletInitCode.length;
address beaconAddress = address(0);
bytes memory slice = _bytes[offset:offset + 20];

// extract beacon address directly from calldata
address beaconAddress;
bytes memory slice = _bytecode[offset:offset + 20];
assembly {
beaconAddress := mload(add(slice, 20))
}
// Compare the extracted beacon address with the disallowed address

require(beaconAddress != address(beacon), "Direct KintoWallet deployment not allowed");
}
}
Expand All @@ -284,10 +284,14 @@ contract KintoWalletFactory is Initializable, UUPSUpgradeable, OwnableUpgradeabl
internal
returns (address)
{
require(amount == msg.value, "amount mismatch");
_preventCreationBytecode(bytecode);
require(amount == msg.value, "Amount mismatch");
require(bytecode.length > 0, "Bytecode is empty");
_preventWalletDeployment(bytecode);

// deploy the contract using `CREATE2`
address created = Create2.deploy(amount, salt, bytecode);
// Assign ownership to the deployer if needed

// assign ownership to newOwner if the contract is Ownable
try OwnableUpgradeable(created).owner() returns (address owner) {
if (owner == address(this)) {
OwnableUpgradeable(created).transferOwnership(newOwner);
Expand Down
43 changes: 17 additions & 26 deletions test/KintoID.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,12 @@ contract KintoIDTest is KYCSignature, AATestScaffolding, UserOp {

/* ============ Burn tests ============ */

function testBurnKYC_RevertWhen_UsingBurn() public {
function testBurn_RevertWhen_UsingBurn() public {
vm.expectRevert("Use burnKYC instead");
_kintoID.burn(1);
}

function test_RevertWhen_BurnIsCalled() public {
function testBurn_RevertWhen_BurnIsCalled() public {
approveKYC(_kycProvider, _user, _userPk);
uint256 tokenIdx = _kintoID.tokenOfOwnerByIndex(_user, 0);
vm.prank(_user);
Expand All @@ -194,49 +194,40 @@ contract KintoIDTest is KYCSignature, AATestScaffolding, UserOp {
}

function testBurnKYC() public {
approveKYC(_kycProvider, _user, _userPk);

IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
uint16[] memory traits = new uint16[](1);
traits[0] = 1;
vm.startPrank(_kycProvider);
_kintoID.mintIndividualKyc(sigdata, traits);
assertEq(_kintoID.isKYC(_user), true);
sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
vm.prank(_kycProvider);
_kintoID.burnKYC(sigdata);
assertEq(_kintoID.balanceOf(_user), 0);
}

function testOnlyProviderCanBurnKYC() public {
function testBurnKYC_WhenCallerIsNotProvider() public {
approveKYC(_kycProvider, _user, _userPk);

IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
uint16[] memory traits = new uint16[](1);
traits[0] = 1;
vm.startPrank(_kycProvider);
_kintoID.mintIndividualKyc(sigdata, traits);
assertEq(_kintoID.isKYC(_user), true);
sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
vm.stopPrank();
vm.startPrank(_user);
vm.expectRevert("Invalid Provider");
vm.startPrank(_user);
_kintoID.burnKYC(sigdata);
}

function testBurnFailsWithoutMinting() public {
function testBurnKYC_WhenUserIsNotKYCd() public {
IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
vm.startPrank(_kycProvider);
vm.expectRevert("Nothing to burn");
vm.prank(_kycProvider);
_kintoID.burnKYC(sigdata);
}

function testBurningTwiceFails() public {
function testBurnKYC_WhenBurningTwice() public {
approveKYC(_kycProvider, _user, _userPk);

IKintoID.SignatureData memory sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
uint16[] memory traits = new uint16[](1);
traits[0] = 1;
vm.startPrank(_kycProvider);
_kintoID.mintIndividualKyc(sigdata, traits);
assertEq(_kintoID.isKYC(_user), true);
sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
vm.prank(_kycProvider);
_kintoID.burnKYC(sigdata);

sigdata = _auxCreateSignature(_kintoID, _user, _userPk, block.timestamp + 1000);
vm.expectRevert("Nothing to burn");
vm.prank(_kycProvider);
_kintoID.burnKYC(sigdata);
}

Expand Down
27 changes: 20 additions & 7 deletions test/KintoWalletFactory.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,26 @@ contract KintoWalletFactoryTest is SharedSetup {
assertEq(Counter(created).count(), 1);
}

function testDeployContract_RevertWhen_CreateWalletThroughDeploy() public {
function testDeployContract_RevertWhen_SenderNotKYCd() public {
vm.prank(_user2);
vm.expectRevert("KYC required");
_walletFactory.deployContract(_owner, 0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
}

function testDeployContract_RevertWhen_AmountMismatch() public {
vm.deal(_owner, 1 ether);
vm.prank(_owner);
vm.expectRevert("Amount mismatch");
_walletFactory.deployContract(_owner, 1 ether + 1, abi.encodePacked(type(Counter).creationCode), bytes32(0));
}

function testDeployContract_RevertWhen_ZeroBytecode() public {
vm.expectRevert("Bytecode is empty");
vm.prank(_owner);
_walletFactory.deployContract(_owner, 0, bytes(""), bytes32(0));
}

function testDeployContract_RevertWhen_CreateWallet() public {
bytes memory initialize = abi.encodeWithSelector(IKintoWallet.initialize.selector, _owner, _owner);
bytes memory bytecode = abi.encodePacked(
type(SafeBeaconProxy).creationCode, abi.encode(address(_walletFactory.beacon()), initialize)
Expand All @@ -175,12 +194,6 @@ contract KintoWalletFactoryTest is SharedSetup {
_walletFactory.deployContract(_owner, 0, bytecode, bytes32(0));
}

function testDeployContract_RevertWhen_SenderNotKYCd() public {
vm.prank(_user2);
vm.expectRevert("KYC required");
_walletFactory.deployContract(_owner, 0, abi.encodePacked(type(Counter).creationCode), bytes32(0));
}

function testFundWallet() public {
vm.prank(_owner);
_walletFactory.fundWallet{value: 1e18}(payable(address(_kintoWallet)));
Expand Down
Loading

0 comments on commit 7239d0a

Please sign in to comment.