diff --git a/contracts/smart-account/modules/BatchedSessionRouterModule.sol b/contracts/smart-account/modules/BatchedSessionRouterModule.sol index 2d17e176..833ef42c 100644 --- a/contracts/smart-account/modules/BatchedSessionRouterModule.sol +++ b/contracts/smart-account/modules/BatchedSessionRouterModule.sol @@ -10,6 +10,8 @@ import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; import "@account-abstraction/contracts/core/Helpers.sol"; import {UserOperation} from "@account-abstraction/contracts/interfaces/UserOperation.sol"; +//import "hardhat/console.sol"; + /** * @title Batched Session Router * @dev Built to process executeBatch and executeBatch_y6U calls diff --git a/contracts/smart-account/modules/SessionValidationModules/ERC721ApprovalSessionValidationModule.sol b/contracts/smart-account/modules/SessionValidationModules/ERC721ApprovalSessionValidationModule.sol index 3a1be569..e2e4c4e5 100644 --- a/contracts/smart-account/modules/SessionValidationModules/ERC721ApprovalSessionValidationModule.sol +++ b/contracts/smart-account/modules/SessionValidationModules/ERC721ApprovalSessionValidationModule.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.17; import "./ISessionValidationModule.sol"; import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -// import "hardhat/console.sol"; +//import "hardhat/console.sol"; -contract ERC721ApprovalSessionValidationModule { +contract ERC721ApprovalSessionValidationModule is ISessionValidationModule { /** * @dev validates if the _op (UserOperation) matches the SessionKey permissions * and that _op has been signed by this SessionKey @@ -20,11 +20,11 @@ contract ERC721ApprovalSessionValidationModule { bytes32 _userOpHash, bytes calldata _sessionKeyData, bytes calldata _sessionKeySignature - ) external view returns (bool) { + ) external view override returns (bool) { address sessionKey = address(bytes20(_sessionKeyData[0:20])); address nftContract = address(bytes20(_sessionKeyData[20:40])); - // we expect _op.callData to be `SmartAccount.executeCall(to, value, calldata)` calldata + // we expect _op.callData to be `SmartAccount.execute(to, value, calldata)` calldata (address tokenAddr, uint256 callValue, ) = abi.decode( _op.callData[4:], // skip selector (address, uint256, bytes) @@ -53,9 +53,41 @@ contract ERC721ApprovalSessionValidationModule { ) == sessionKey; } + function validateSessionParams( + address destinationContract, + uint256 callValue, + bytes calldata _funcCallData, + bytes calldata _sessionKeyData, + bytes calldata /*_callSpecificData*/ + ) external view virtual override returns (address) { + + //alternative way of decoding data + address sessionKey = address(bytes20(_sessionKeyData[0:20])); + address nftContract = address(bytes20(_sessionKeyData[20:40])); + + //using the _funcCallData which is a clean calldata of the action we're validating + bytes4 selector = bytes4(_funcCallData[0:4]); + //[4:36] is the address of the operator + bool approved = (uint256(bytes32(_funcCallData[36:])) == 1); + + if (destinationContract != nftContract) { + revert("ERC721SV Wrong NFT contract"); + } + if (callValue > 0) { + revert("ERC721SV Non Zero Value"); + } + if (selector != bytes4(0xa22cb465)) { + revert("ERC721SV Not Approval For All"); + } + if (!approved) { + revert("ERC721SV False value"); + } + return sessionKey; + } + function _getApprovalForAllData( bytes calldata _approvalForAllCalldata - ) internal view returns (bytes4 selector, bool approved) { + ) internal pure returns (bytes4 selector, bool approved) { //first 32 bytes is the length of bytes array selector = bytes4(_approvalForAllCalldata[32:36]); //36:68 is the address of the operator diff --git a/test/bundler-integration/module/BatchedSessionRouter.Module.specs.ts b/test/bundler-integration/module/BatchedSessionRouter.Module.specs.ts index 1ffca91d..abad0849 100644 --- a/test/bundler-integration/module/BatchedSessionRouter.Module.specs.ts +++ b/test/bundler-integration/module/BatchedSessionRouter.Module.specs.ts @@ -16,6 +16,7 @@ import { } from "../../utils/setupHelper"; import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers"; import { BundlerTestEnvironment } from "../environment/bundlerEnvironment"; +import { hexZeroPad, hexConcat } from "ethers/lib/utils"; describe("SessionKey: Session Router (via Bundler)", async () => { let [deployer, smartAccountOwner, charlie, verifiedSigner, sessionKey] = @@ -79,7 +80,6 @@ describe("SessionKey: Session Router (via Bundler)", async () => { }); await mockToken.mint(userSA.address, ethers.utils.parseEther("1000000")); - // deploy forward flow module and enable it in the smart account const sessionKeyManager = await ( await ethers.getContractFactory("SessionKeyManager") ).deploy(); @@ -121,6 +121,13 @@ describe("SessionKey: Session Router (via Bundler)", async () => { await ethers.getContractFactory("MockProtocolSVM") ).deploy(); + const mockNFT = await ( + await ethers.getContractFactory("MockNFT") + ).deploy(); + const erc721ApprovalSVM = await ( + await ethers.getContractFactory("ERC721ApprovalSessionValidationModule") + ).deploy(); + const { sessionKeyData, leafData } = await getERC20SessionKeyParams( sessionKey.address, mockToken.address, @@ -142,13 +149,28 @@ describe("SessionKey: Session Router (via Bundler)", async () => { 0, mockProtocolSVModule.address ); + + const sessionKeyData3 = hexConcat([ + hexZeroPad(sessionKey.address, 20), + hexZeroPad(mockNFT.address, 20), + ]); + + const leafData3 = hexConcat([ + hexZeroPad(ethers.utils.hexlify(0), 6), + hexZeroPad(ethers.utils.hexlify(0), 6), + hexZeroPad(erc721ApprovalSVM.address, 20), + sessionKeyData3, + ]); // build a big tree const leaves = [ethers.utils.keccak256(leafData)]; - for (let i = 0; i < 9999; i++) { + for (let i = 0; i < 9998; i++) { if (i === 4988) { leaves.push(ethers.utils.keccak256(leafData2)); } + if (i === 2517) { + leaves.push(ethers.utils.keccak256(leafData3)); + } leaves.push(ethers.utils.keccak256(ethers.utils.randomBytes(32))); } @@ -178,10 +200,14 @@ describe("SessionKey: Session Router (via Bundler)", async () => { leafData: leafData, sessionKeyData2: sessionKeyData2, leafData2: leafData2, + sessionKeyData3: sessionKeyData3, + leafData3: leafData3, merkleTree: merkleTree, sessionRouter: sessionRouter, mockProtocol: mockProtocol, mockProtocolSVM: mockProtocolSVModule, + mockNFT: mockNFT, + erc721ApprovalSVM: erc721ApprovalSVM, }; } ); @@ -201,12 +227,17 @@ describe("SessionKey: Session Router (via Bundler)", async () => { mockToken, sessionKeyData2, leafData2, + sessionKeyData3, + leafData3, + mockNFT, + erc721ApprovalSVM } = await setupTests(); const tokenAmountToTransfer = ethers.utils.parseEther("5.7534"); const MockProtocol = await ethers.getContractFactory("MockProtocol"); const IERC20 = await ethers.getContractFactory("ERC20"); const SmartAccount = await ethers.getContractFactory("SmartAccount"); + const Erc721 = await ethers.getContractFactory("MockNFT"); const approveCallData = IERC20.interface.encodeFunctionData("approve", [ mockProtocol.address, @@ -216,12 +247,18 @@ describe("SessionKey: Session Router (via Bundler)", async () => { "interact", [mockToken.address, tokenAmountToTransfer] ); + + const approveNFTCalldata = Erc721.interface.encodeFunctionData( + "setApprovalForAll", + [charlie.address, true] + ); + const executeBatchData = SmartAccount.interface.encodeFunctionData( "executeBatch_y6U", [ - [mockToken.address, mockProtocol.address], - [0, 0], - [approveCallData, interactCallData], + [mockToken.address, mockProtocol.address, mockNFT.address], + [0, 0, 0], + [approveCallData, interactCallData, approveNFTCalldata], ] ); @@ -229,7 +266,7 @@ describe("SessionKey: Session Router (via Bundler)", async () => { { sender: userSA.address, callData: executeBatchData, - preVerificationGas: 75000, + preVerificationGas: 85000, }, sessionKey, entryPoint, @@ -271,6 +308,14 @@ describe("SessionKey: Session Router (via Bundler)", async () => { merkleTree.getHexProof(ethers.utils.keccak256(leafData2)), "0x", ], + [ + 0, + 0, + erc721ApprovalSVM.address, + sessionKeyData3, + merkleTree.getHexProof(ethers.utils.keccak256(leafData3)), + "0x", + ], ], signatureOverUserOpHashAndModuleAddress, ] @@ -287,5 +332,6 @@ describe("SessionKey: Session Router (via Bundler)", async () => { expect(await mockToken.balanceOf(mockProtocol.address)).to.equal( tokenAmountToTransfer ); + expect(await mockNFT.isApprovedForAll(userSA.address, charlie.address)).to.equal(true); }); }); diff --git a/test/module/SessionValidationModules/ERC721ApprovalSessionValidation.Module.specs.ts b/test/module/SessionValidationModules/ERC721ApprovalSessionValidation.Module.specs.ts index 552e84df..e3516b80 100644 --- a/test/module/SessionValidationModules/ERC721ApprovalSessionValidation.Module.specs.ts +++ b/test/module/SessionValidationModules/ERC721ApprovalSessionValidation.Module.specs.ts @@ -14,7 +14,7 @@ import { getSmartAccountWithModule, getVerifyingPaymaster, } from "../../utils/setupHelper"; -import { hexZeroPad, hexConcat} from "ethers/lib/utils"; +import { hexZeroPad, hexConcat } from "ethers/lib/utils"; describe("SessionKey: ERC721 Approval Session Validation Module", async () => { const [