Skip to content
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ temp
coverage.json
*.tsbuildinfo

integration-tests/result/output.xml

env.yml
env.yaml
.aws-sam/
Expand Down
54 changes: 54 additions & 0 deletions integration-tests/test/eth-l2/boba_aa_sponsoring_fee.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,5 +171,59 @@ describe('Sponsoring Tx\n', async () => {
expect(postUserBalance).to.eq(preUserBalance)
expect(postPaymasterDeposit).to.eq(prePaymasterDeposit.sub(logEP.args.actualGasCost))
})

it('should not be able to submit a userOp to the bundler and trigger tx when signature expired', async () => {
const validUntil = (await env.l2Provider.getBlock('latest')).timestamp - 300
const validAfter = (await env.l2Provider.getBlock('latest')).timestamp - 600
const op = await accountAPI.createSignedUserOp({
target: recipient.address,
data: recipient.interface.encodeFunctionData('something', ['hello']),
})

// add preverificaiton gas to account for paymaster signature
op.paymasterAndData = hexConcat([VerifyingPaymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [validUntil, validAfter]), '0x' + '00'.repeat(65)])
op.preVerificationGas = BigNumber.from(await op.preVerificationGas).add(3000)
const hash = await VerifyingPaymaster.getHash(op, validUntil, validAfter)
const sig = await offchainSigner.signMessage(utils.arrayify(hash))

op.paymasterAndData = hexConcat([VerifyingPaymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [validUntil, validAfter]), sig])
const res = await VerifyingPaymaster.parsePaymasterAndData(op.paymasterAndData)

expect(res.signature).to.eq(sig)
expect(res.validAfter).to.eq(validAfter)
expect(res.validUntil).to.eq(validUntil)
signedOp = await accountAPI.signUserOp(op)

await expect(bundlerProvider.sendUserOpToBundler(signedOp)).to.be.rejectedWith(
Error, /expires too soon/
)
})

it('should not be able to submit a userOp to the bundler and trigger tx when signature is not valid yet', async () => {
const validUntil = (await env.l2Provider.getBlock('latest')).timestamp + 800
const validAfter = (await env.l2Provider.getBlock('latest')).timestamp + 600
const op = await accountAPI.createSignedUserOp({
target: recipient.address,
data: recipient.interface.encodeFunctionData('something', ['hello']),
})

// add preverificaiton gas to account for paymaster signature
op.paymasterAndData = hexConcat([VerifyingPaymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [validUntil, validAfter]), '0x' + '00'.repeat(65)])
op.preVerificationGas = BigNumber.from(await op.preVerificationGas).add(3000)
const hash = await VerifyingPaymaster.getHash(op, validUntil, validAfter)
const sig = await offchainSigner.signMessage(utils.arrayify(hash))

op.paymasterAndData = hexConcat([VerifyingPaymaster.address, defaultAbiCoder.encode(['uint48', 'uint48'], [validUntil, validAfter]), sig])
const res = await VerifyingPaymaster.parsePaymasterAndData(op.paymasterAndData)

expect(res.signature).to.eq(sig)
expect(res.validAfter).to.eq(validAfter)
expect(res.validUntil).to.eq(validUntil)
signedOp = await accountAPI.signUserOp(op)

await expect(bundlerProvider.sendUserOpToBundler(signedOp)).to.be.rejectedWith(
Error, /not valid yet/
)
})
})
})
1 change: 1 addition & 0 deletions packages/boba/bundler/src/modules/Types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export enum ValidationErrors {
InsufficientStake = -32505,
UnsupportedSignatureAggregator = -32506,
InvalidSignature = -32507,
NotValidYet = -32508,
}

export enum ExecutionErrors {
Expand Down
25 changes: 14 additions & 11 deletions packages/boba/bundler/src/modules/ValidationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,17 +162,20 @@ export class ValidationManager {
'Invalid UserOp signature or paymaster signature',
ValidationErrors.InvalidSignature
)
//TODO
// requireCond(
// res.returnInfo.validUntil == null ||
// res.returnInfo.validUntil + 30 < Date.now() / 1000,
// 'expires too soon',
// ValidationErrors.ExpiresShortly
// )
console.log(`res.aggregatorInfo ${res.aggregatorInfo}`)
console.log(res.aggregatorInfo.addr)
console.log(res.aggregatorInfo.stake)
console.log(res.aggregatorInfo.unstakeDelaySec)
requireCond(
res.returnInfo.validUntil == null ||
res.returnInfo.validUntil > (Date.now() / 1000) + 30,
'expires too soon',
ValidationErrors.ExpiresShortly,
)
requireCond(
res.returnInfo.validAfter == null ||
// not adding "buffer" here
res.returnInfo.validAfter < Date.now() / 1000,
'not valid yet',
ValidationErrors.NotValidYet,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the doc with the new error code (https://github.com/bobanetwork/boba/pull/778)

this is better, but just wondering if we should squeeze this into the -32503 error code? (referencing the error codes in the spec https://github.com/ethereum/EIPs/blob/master/EIPS/eip-4337.md)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point!

Well both are fine for me, advantage would be to stick more to the original spec but disadvantage would be that you don't know if the signature expired or will be valid in future when merging them together (and the error message might be misleading too except we change that altogether).

ExpiresShortly = -32503

)

if (
res.aggregatorInfo.addr !== AddressZero &&
!BigNumber.from(0).eq(res.aggregatorInfo.stake) &&
Expand Down
33 changes: 28 additions & 5 deletions packages/boba/bundler/test/ValidateManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { isGeth } from '../src/utils'
import { TestRecursionAccount__factory } from '../dist/src/types/factories/contracts/tests/TestRecursionAccount__factory'
// import { resolveNames } from './testUtils'
import { UserOperation } from '../src/modules/Types'
import { BytesLike } from "ethers";

const cEmptyUserOp: UserOperation = {
sender: AddressZero,
Expand All @@ -51,9 +52,9 @@ describe('#ValidationManager', () => {
let rulesAccount: TestRulesAccount
let storageAccount: TestStorageAccount

async function testUserOp (validateRule: string = '', pmRule?: string, initFunc?: string, factoryAddress = opcodeFactory.address): Promise<ValidateUserOpResult & { userOp: UserOperation }> {
const userOp = await createTestUserOp(validateRule, pmRule, initFunc, factoryAddress)
return { userOp, ...await vm.validateUserOp(userOp) }
async function testUserOp (validateRule: string = '', pmRule?: string, initFunc?: string, factoryAddress = opcodeFactory.address, paymasterData?: BytesLike[]): Promise<ValidateUserOpResult & { userOp: UserOperation }> {
const userOp = await createTestUserOp(validateRule, pmRule, initFunc, factoryAddress, paymasterData)
return { userOp, ...await vm.validateUserOp(userOp) }
}

async function testExistingUserOp (validateRule: string = '', pmRule = ''): Promise<ValidateUserOpResult & { userOp: UserOperation }> {
Expand All @@ -75,7 +76,7 @@ describe('#ValidationManager', () => {
}
}

async function createTestUserOp (validateRule: string = '', pmRule?: string, initFunc?: string, factoryAddress = opcodeFactory.address): Promise<UserOperation> {
async function createTestUserOp (validateRule: string = '', pmRule?: string, initFunc?: string, factoryAddress = opcodeFactory.address, paymasterData?: BytesLike[]): Promise<UserOperation> {
if (initFunc === undefined) {
initFunc = opcodeFactory.interface.encodeFunctionData('create', [''])
}
Expand All @@ -84,7 +85,11 @@ describe('#ValidationManager', () => {
factoryAddress,
initFunc
])
const paymasterAndData = pmRule == null ? '0x' : hexConcat([paymaster.address, Buffer.from(pmRule)])

if (!paymasterData || !paymasterData.length) {
paymasterData.push(Buffer.from(pmRule))
}
const paymasterAndData = pmRule == null ? '0x' : hexConcat([paymaster.address, ...paymasterData ])
const signature = hexlify(Buffer.from(validateRule))
const callinitCodeForAddr = await provider.call({
to: factoryAddress,
Expand Down Expand Up @@ -184,6 +189,24 @@ describe('#ValidationManager', () => {
.catch(e => e)
).to.match(/unstaked account accessed/)
})
it('should fail with expired signature in validation', async () => {
const currBlockTimestamp = (await provider.getBlock()).timestamp
const validUntil = currBlockTimestamp - 300
const validAfter = currBlockTimestamp - 600
const paymasterData = [defaultAbiCoder.encode(['uint48', 'uint48'], [validUntil, validAfter]), '0x' + '00'.repeat(65)]

expect(await testUserOp('expired-sig', undefined, undefined, undefined, paymasterData)
.catch(e => e.message)).to.match(/expires too soon/)
})
it('should fail with signature that is only valid in future in validation', async () => {
const currBlockTimestamp = (await provider.getBlock()).timestamp
const validUntil = currBlockTimestamp + 800
const validAfter = currBlockTimestamp + 600
const paymasterData = [defaultAbiCoder.encode(['uint48', 'uint48'], [validUntil, validAfter]), '0x' + '00'.repeat(65)]

expect(await testUserOp('not-yet-valid-sig', undefined, undefined, undefined, paymasterData)
.catch(e => e.message)).to.match(/not valid yet/)
})

it('account succeeds referencing its own balance (after wallet creation)', async () => {
await testExistingUserOp('balance-self')
Expand Down