From 95a378640e21f89ac08413dc89415d183cc4d537 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 16:46:06 +0300 Subject: [PATCH 1/5] initial paymaster test flows --- core/state_processor_rip7560.go | 32 ++++++++++++---- tests/rip7560/paymaster_test.go | 67 +++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 8 deletions(-) create mode 100644 tests/rip7560/paymaster_test.go diff --git a/core/state_processor_rip7560.go b/core/state_processor_rip7560.go index 208e4609de..0ceb83448e 100644 --- a/core/state_processor_rip7560.go +++ b/core/state_processor_rip7560.go @@ -37,9 +37,16 @@ func UnpackValidationData(validationData []byte) (authorizerMagic uint64, validU } func UnpackPaymasterValidationReturn(paymasterValidationReturn []byte) (validationData, context []byte) { + if len(paymasterValidationReturn) < 96 { + return nil, nil + } validationData = paymasterValidationReturn[0:32] //2nd bytes32 is ignored (its an offset value) contextLen := new(big.Int).SetBytes(paymasterValidationReturn[64:96]) + if uint64(len(paymasterValidationReturn)) < 96+contextLen.Uint64() { + return nil, nil + } + context = paymasterValidationReturn[96 : 96+contextLen.Uint64()] return } @@ -133,17 +140,17 @@ func BuyGasRip7560Transaction(st *types.Rip7560AccountAbstractionTx, state vm.St mgval = mgval.Mul(mgval, gasFeeCap) balanceCheck := new(uint256.Int).Set(mgval) - chargeFrom := *st.Sender + chargeFrom := st.Sender - if len(st.PaymasterData) >= 20 { - chargeFrom = [20]byte(st.PaymasterData[:20]) + if st.Paymaster != nil { + chargeFrom = st.Paymaster } - if have, want := state.GetBalance(chargeFrom), balanceCheck; have.Cmp(want) < 0 { + if have, want := state.GetBalance(*chargeFrom), balanceCheck; have.Cmp(want) < 0 { return fmt.Errorf("%w: address %v have %v want %v", ErrInsufficientFunds, chargeFrom.Hex(), have, want) } - state.SubBalance(chargeFrom, mgval, 0) + state.SubBalance(*chargeFrom, mgval, 0) return nil } @@ -210,6 +217,9 @@ func ApplyRip7560ValidationPhases(chainConfig *params.ChainConfig, bc ChainConte } paymasterContext, pmValidationUsedGas, pmValidAfter, pmValidUntil, err := applyPaymasterValidationFrame(tx, chainConfig, signingHash, evm, gp, statedb, header) + if err != nil { + return nil, err + } vpr := &ValidationPhaseResult{ Tx: tx, TxHash: tx.Hash(), @@ -241,6 +251,9 @@ func applyPaymasterValidationFrame(tx *types.Transaction, chainConfig *params.Ch if err != nil { return nil, 0, 0, 0, err } + if resultPm.Failed() { + return nil, 0, 0, 0, resultPm.Err + } statedb.IntermediateRoot(true) if resultPm.Failed() { return nil, 0, 0, 0, errors.New("paymaster validation failed - invalid transaction") @@ -370,10 +383,10 @@ func prepareAccountValidationMessage(baseTx *types.Transaction, chainConfig *par func preparePaymasterValidationMessage(baseTx *types.Transaction, config *params.ChainConfig, signingHash common.Hash) (*Message, error) { tx := baseTx.Rip7560TransactionData() - if len(tx.PaymasterData) < 20 { + paymasterAddress := tx.Paymaster + if paymasterAddress == nil { return nil, nil } - var paymasterAddress common.Address = [20]byte(tx.PaymasterData[0:20]) jsondata := `[ {"type":"function","name":"validatePaymasterTransaction","inputs": [{"name": "version","type": "uint256"},{"name": "txHash","type": "bytes32"},{"name": "transaction","type": "bytes"}]} ]` @@ -387,7 +400,7 @@ func preparePaymasterValidationMessage(baseTx *types.Transaction, config *params } return &Message{ From: config.EntryPointAddress, - To: &paymasterAddress, + To: paymasterAddress, Value: big.NewInt(0), GasLimit: tx.PaymasterGas, GasPrice: tx.GasFeeCap, @@ -470,6 +483,9 @@ func validatePaymasterReturnData(data []byte) (context []byte, validAfter, valid return nil, 0, 0, errors.New("invalid paymaster return data length") } validationData, context := UnpackPaymasterValidationReturn(data) + if validationData == nil { + return nil, 0, 0, errors.New("invalid paymaster return data") + } magicExpected, validAfter, validUntil := UnpackValidationData(validationData) if magicExpected != MAGIC_VALUE_PAYMASTER { return nil, 0, 0, errors.New("paymaster did not return correct MAGIC_VALUE") diff --git a/tests/rip7560/paymaster_test.go b/tests/rip7560/paymaster_test.go new file mode 100644 index 0000000000..bb2cd6c670 --- /dev/null +++ b/tests/rip7560/paymaster_test.go @@ -0,0 +1,67 @@ +package rip7560 + +import ( + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core" + "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/core/vm" + "math/big" + "slices" + "testing" +) + +var DEFAULT_PAYMASTER = common.HexToAddress("0xaaaaaaaaaabbbbbbbbbbccccccccccdddddddddd") + +func TestPaymasterValidationFailure_nobalance(t *testing.T) { + + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), createCode(vm.PUSH0, vm.DUP1, vm.REVERT), 1), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "insufficient funds for gas * price + value: address 0xaaAaaAAAAAbBbbbbBbBBCCCCcCCCcCdddDDDdddd have 1 want 1000000000000000000") +} + +func TestPaymasterValidationFailure_oog(t *testing.T) { + + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), createCode(vm.PUSH0, vm.DUP1, vm.REVERT), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "out of gas") +} +func TestPaymasterValidationFailure_revert(t *testing.T) { + + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), createCode(vm.PUSH0, vm.DUP1, vm.REVERT), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: uint64(1000000000), + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + PaymasterGas: 1000000000, + }, "execution reverted") +} + +func asBytes32(a int) []byte { + return common.LeftPadBytes(big.NewInt(int64(a)).Bytes(), 32) +} +func paymasterReturnValue(magic, validAfter, validUntil uint64, context []byte) []byte { + validationData := core.PackValidationData(magic, validUntil, validAfter) + //manual encode (bytes32 validationData, bytes context) + return slices.Concat( + common.LeftPadBytes(validationData, 32), + asBytes32(64), + asBytes32(len(context)), + context) +} + +func TestPaymasterValidationFailure_unparseable_return_value(t *testing.T) { + + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), createAccountCode(), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + PaymasterGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "invalid paymaster return data") +} From 7331b96c2719023313f19e131d249b65149c76d4 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 14:17:25 +0300 Subject: [PATCH 2/5] returnWithData, revertWithData arbitrary-length return or revert (currently, can't test it, since we check "error", and actual revert data is hidden. will be probably needed when testing paymaster.) --- tests/rip7560/rip7560TestUtils.go | 38 +++++++++++++++++++++++-------- tests/rip7560/validation_test.go | 15 +++++++----- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/tests/rip7560/rip7560TestUtils.go b/tests/rip7560/rip7560TestUtils.go index 1e50d9b7fb..a60b473066 100644 --- a/tests/rip7560/rip7560TestUtils.go +++ b/tests/rip7560/rip7560TestUtils.go @@ -83,20 +83,38 @@ func (tt *testContextBuilder) withCode(addr string, code []byte, balance int64) return tt } -// generate the code to return the given byte array (up to 32 bytes) -func returnData(data []byte) []byte { - datalen := len(data) - if datalen > 32 { - panic(fmt.Errorf("data length is too big %v", data)) +// create code to copy data into memory at the given offset +// NOTE: if data is not in 32-byte multiples, it will override the next bytes +// used by RETURN/REVERT +func copyToMemory(data []byte, offset uint) []byte { + ret := []byte{} + for len(data) > 32 { + ret = append(ret, createCode(vm.PUSH32, data[0:32], vm.PUSH2, uint16(offset), vm.MSTORE)...) + data = data[32:] + offset = offset + 32 } - PUSHn := byte(int(vm.PUSH0) + datalen) - ret := createCode(PUSHn, data, vm.PUSH0, vm.MSTORE, vm.PUSH1, datalen, vm.PUSH1, 0, vm.RETURN) + if len(data) > 0 { + PUSHn := byte(int(vm.PUSH0) + len(data)) + ret = append(ret, createCode(PUSHn, data, vm.PUSH2, uint16(offset), vm.MSTORE)...) + } + return ret +} + +// revert with given data +func revertWithData(data []byte) []byte { + ret := append(copyToMemory(data, 0), createCode(vm.PUSH2, uint16(len(data)), vm.PUSH0, vm.REVERT)...) + return ret +} + +// generate the code to return the given byte array (up to 32 bytes) +func returnWithData(data []byte) []byte { + ret := append(copyToMemory(data, 0), createCode(vm.PUSH2, uint16(len(data)), vm.PUSH0, vm.RETURN)...) return ret } func createAccountCode() []byte { - return returnData(core.PackValidationData(core.MAGIC_VALUE_SENDER, 0, 0)) + return returnWithData(core.PackValidationData(core.MAGIC_VALUE_SENDER, 0, 0)) } // create EVM code from OpCode, byte and []bytes @@ -115,9 +133,11 @@ func createCode(items ...interface{}) []byte { buffer.Write(v) case int8: buffer.WriteByte(byte(v)) + case uint16: + buffer.Write([]byte{byte(v >> 8), byte(v)}) case int: if v >= 256 { - panic(fmt.Errorf("int defaults to int8 (byte). int16, etc: %v", v)) + panic(fmt.Errorf("int defaults to int8 (byte). use int16, etc: %v", v)) } buffer.WriteByte(byte(v)) default: diff --git a/tests/rip7560/validation_test.go b/tests/rip7560/validation_test.go index cdbd5d3c24..1afec6c7f0 100644 --- a/tests/rip7560/validation_test.go +++ b/tests/rip7560/validation_test.go @@ -5,6 +5,7 @@ import ( "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/vm" "github.com/ethereum/go-ethereum/tests" + "github.com/status-im/keycard-go/hexutils" "github.com/stretchr/testify/assert" "math/big" "testing" @@ -43,7 +44,7 @@ func TestValidationFailure_no_balance(t *testing.T) { } func TestValidationFailure_sigerror(t *testing.T) { - handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, returnData(core.PackValidationData(core.MAGIC_VALUE_SIGFAIL, 0, 0)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, returnWithData(core.PackValidationData(core.MAGIC_VALUE_SIGFAIL, 0, 0)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), }, "account signature error") @@ -104,9 +105,11 @@ func TestValidationFailure_account_revert(t *testing.T) { }, "execution reverted") } -func TestValidationFailure_account_out_of_range(t *testing.T) { +func TestValidationFailure_account_revert_with_reason(t *testing.T) { + // cast abi-encode 'Error(string)' hello + reason := hexutils.HexToBytes("0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000568656c6c6f000000000000000000000000000000000000000000000000000000") handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, - createCode(vm.PUSH0, vm.DUP1, vm.REVERT), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + revertWithData(reason), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), }, "execution reverted") @@ -114,7 +117,7 @@ func TestValidationFailure_account_out_of_range(t *testing.T) { func TestValidationFailure_account_wrong_return_length(t *testing.T) { handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, - returnData([]byte{1, 2, 3}), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + returnWithData([]byte{1, 2, 3}), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), }, "invalid account return data length") @@ -122,7 +125,7 @@ func TestValidationFailure_account_wrong_return_length(t *testing.T) { func TestValidationFailure_account_no_return_value(t *testing.T) { handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, - returnData([]byte{}), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + returnWithData([]byte{}), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), }, "invalid account return data length") @@ -131,7 +134,7 @@ func TestValidationFailure_account_no_return_value(t *testing.T) { func TestValidationFailure_account_wrong_return_value(t *testing.T) { // create buffer of 32 byte array handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, - returnData(make([]byte, 32)), + returnWithData(make([]byte, 32)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), From d804c44262f8b2f9781e71d03b1038c38f6ab5c5 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 20:49:00 +0300 Subject: [PATCH 3/5] test paymaster validations --- core/state_processor_rip7560.go | 7 ++-- tests/rip7560/paymaster_test.go | 56 +++++++++++++++++++++++-------- tests/rip7560/rip7560TestUtils.go | 34 +++++++++++++++++++ tests/rip7560/validation_test.go | 4 +-- 4 files changed, 83 insertions(+), 18 deletions(-) diff --git a/core/state_processor_rip7560.go b/core/state_processor_rip7560.go index 0ceb83448e..aa815ad76d 100644 --- a/core/state_processor_rip7560.go +++ b/core/state_processor_rip7560.go @@ -447,10 +447,10 @@ func preparePostOpMessage(vpr *ValidationPhaseResult, chainConfig *params.ChainC if err != nil { return nil, err } - var paymasterAddress common.Address = [20]byte(tx.PaymasterData[0:20]) + var paymasterAddress = tx.Paymaster return &Message{ From: chainConfig.EntryPointAddress, - To: &paymasterAddress, + To: paymasterAddress, Value: big.NewInt(0), GasLimit: tx.PaymasterGas - executionResult.UsedGas, GasPrice: tx.GasFeeCap, @@ -490,6 +490,9 @@ func validatePaymasterReturnData(data []byte) (context []byte, validAfter, valid if magicExpected != MAGIC_VALUE_PAYMASTER { return nil, 0, 0, errors.New("paymaster did not return correct MAGIC_VALUE") } + if len(context) > PAYMASTER_MAX_CONTEXT_SIZE { + return nil, 0, 0, errors.New("paymaster context too large") + } return context, validAfter, validUntil, nil } diff --git a/tests/rip7560/paymaster_test.go b/tests/rip7560/paymaster_test.go index bb2cd6c670..4d9a37ce4e 100644 --- a/tests/rip7560/paymaster_test.go +++ b/tests/rip7560/paymaster_test.go @@ -6,7 +6,6 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" "math/big" - "slices" "testing" ) @@ -42,19 +41,6 @@ func TestPaymasterValidationFailure_revert(t *testing.T) { }, "execution reverted") } -func asBytes32(a int) []byte { - return common.LeftPadBytes(big.NewInt(int64(a)).Bytes(), 32) -} -func paymasterReturnValue(magic, validAfter, validUntil uint64, context []byte) []byte { - validationData := core.PackValidationData(magic, validUntil, validAfter) - //manual encode (bytes32 validationData, bytes context) - return slices.Concat( - common.LeftPadBytes(validationData, 32), - asBytes32(64), - asBytes32(len(context)), - context) -} - func TestPaymasterValidationFailure_unparseable_return_value(t *testing.T) { handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). @@ -65,3 +51,45 @@ func TestPaymasterValidationFailure_unparseable_return_value(t *testing.T) { Paymaster: &DEFAULT_PAYMASTER, }, "invalid paymaster return data") } + +func TestPaymasterValidationFailure_wrong_magic(t *testing.T) { + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), returnWithData(paymasterReturnValue(1, 2, 3, []byte{})), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + PaymasterGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "paymaster did not return correct MAGIC_VALUE") +} + +func TestPaymasterValidationFailure_contextTooLarge(t *testing.T) { + //paymaster returning huge context. + // first word is magic return value + // 2nd word is offset (fixed 64) + // 3rd word is length of context (max+1) + // then we return the total length of above (context itself is uninitialized string of max+1 zeroes) + pmCode := createCode( + //vm.PUSH1, 1, vm.PUSH0, vm.RETURN, + copyToMemory(core.PackValidationData(core.MAGIC_VALUE_PAYMASTER, 0, 0), 0), + copyToMemory(asBytes32(64), 32), + copyToMemory(asBytes32(core.PAYMASTER_MAX_CONTEXT_SIZE+1), 64), + push(core.PAYMASTER_MAX_CONTEXT_SIZE+96+1), vm.PUSH0, vm.RETURN) + + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), pmCode, DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + PaymasterGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "paymaster context too large") +} + +func TestPaymasterValidation_ok(t *testing.T) { + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), returnWithData(paymasterReturnValue(core.MAGIC_VALUE_PAYMASTER, 0, 0, []byte{})), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + PaymasterGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "ok") +} diff --git a/tests/rip7560/rip7560TestUtils.go b/tests/rip7560/rip7560TestUtils.go index a60b473066..82c46650dd 100644 --- a/tests/rip7560/rip7560TestUtils.go +++ b/tests/rip7560/rip7560TestUtils.go @@ -11,6 +11,7 @@ import ( "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/status-im/keycard-go/hexutils" "math/big" + "slices" "testing" ) @@ -83,6 +84,23 @@ func (tt *testContextBuilder) withCode(addr string, code []byte, balance int64) return tt } +// generate a push opcode and its following constant value +func push(n int) []byte { + if n < 0 { + panic("attempt to push negative") + } + if n < 256 { + return createCode(vm.PUSH1, byte(n)) + } + if n < 65536 { + return createCode(vm.PUSH2, byte(n>>8), byte(n)) + } + if n < 1<<32 { + return createCode(vm.PUSH4, byte(n>>24), byte(n>>16), byte(n>>8), byte(n)) + } + panic("larger number") +} + // create code to copy data into memory at the given offset // NOTE: if data is not in 32-byte multiples, it will override the next bytes // used by RETURN/REVERT @@ -135,6 +153,8 @@ func createCode(items ...interface{}) []byte { buffer.WriteByte(byte(v)) case uint16: buffer.Write([]byte{byte(v >> 8), byte(v)}) + case uint32: + buffer.Write([]byte{byte(v >> 24), byte(v >> 16), byte(v >> 8), byte(v)}) case int: if v >= 256 { panic(fmt.Errorf("int defaults to int8 (byte). use int16, etc: %v", v)) @@ -148,3 +168,17 @@ func createCode(items ...interface{}) []byte { return buffer.Bytes() } + +func asBytes32(a int) []byte { + return common.LeftPadBytes(big.NewInt(int64(a)).Bytes(), 32) +} + +func paymasterReturnValue(magic, validUntil, validAfter uint64, context []byte) []byte { + validationData := core.PackValidationData(magic, validUntil, validAfter) + //manual encode (bytes32 validationData, bytes context) + return slices.Concat( + common.LeftPadBytes(validationData, 32), + asBytes32(64), + asBytes32(len(context)), + context) +} diff --git a/tests/rip7560/validation_test.go b/tests/rip7560/validation_test.go index 1afec6c7f0..c7a75fdc91 100644 --- a/tests/rip7560/validation_test.go +++ b/tests/rip7560/validation_test.go @@ -53,7 +53,7 @@ func TestValidationFailure_sigerror(t *testing.T) { func TestValidationFailure_validAfter(t *testing.T) { handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, - returnData(core.PackValidationData(core.MAGIC_VALUE_SENDER, 300, 200)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + returnWithData(core.PackValidationData(core.MAGIC_VALUE_SENDER, 300, 200)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), }, "RIP-7560 transaction validity not reached yet") @@ -62,7 +62,7 @@ func TestValidationFailure_validAfter(t *testing.T) { func TestValidationFailure_validUntil(t *testing.T) { handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, - returnData(core.PackValidationData(core.MAGIC_VALUE_SENDER, 1, 0)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + returnWithData(core.PackValidationData(core.MAGIC_VALUE_SENDER, 1, 0)), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ ValidationGas: uint64(1000000000), GasFeeCap: big.NewInt(1000000000), }, "RIP-7560 transaction validity expired") From 604c76341eebec28831c297e230bc65d4ac6feb1 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 22:11:30 +0300 Subject: [PATCH 4/5] paymaster time-range checking --- core/state_processor_rip7560.go | 2 +- tests/rip7560/paymaster_test.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/core/state_processor_rip7560.go b/core/state_processor_rip7560.go index aa815ad76d..9e0333ed4d 100644 --- a/core/state_processor_rip7560.go +++ b/core/state_processor_rip7560.go @@ -486,7 +486,7 @@ func validatePaymasterReturnData(data []byte) (context []byte, validAfter, valid if validationData == nil { return nil, 0, 0, errors.New("invalid paymaster return data") } - magicExpected, validAfter, validUntil := UnpackValidationData(validationData) + magicExpected, validUntil, validAfter := UnpackValidationData(validationData) if magicExpected != MAGIC_VALUE_PAYMASTER { return nil, 0, 0, errors.New("paymaster did not return correct MAGIC_VALUE") } diff --git a/tests/rip7560/paymaster_test.go b/tests/rip7560/paymaster_test.go index 4d9a37ce4e..e6a026d276 100644 --- a/tests/rip7560/paymaster_test.go +++ b/tests/rip7560/paymaster_test.go @@ -84,6 +84,26 @@ func TestPaymasterValidationFailure_contextTooLarge(t *testing.T) { }, "paymaster context too large") } +func TestPaymasterValidationFailure_validAfter(t *testing.T) { + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), returnWithData(paymasterReturnValue(core.MAGIC_VALUE_PAYMASTER, 300, 200, []byte{})), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + PaymasterGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "RIP-7560 transaction validity not reached yet") +} + +func TestPaymasterValidationFailure_validUntil(t *testing.T) { + handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). + withCode(DEFAULT_PAYMASTER.String(), returnWithData(paymasterReturnValue(core.MAGIC_VALUE_PAYMASTER, 1, 0, []byte{})), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ + ValidationGas: 1000000000, + PaymasterGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + Paymaster: &DEFAULT_PAYMASTER, + }, "RIP-7560 transaction validity expired") +} + func TestPaymasterValidation_ok(t *testing.T) { handleTransaction(newTestContextBuilder(t).withCode(DEFAULT_SENDER, createAccountCode(), 0). withCode(DEFAULT_PAYMASTER.String(), returnWithData(paymasterReturnValue(core.MAGIC_VALUE_PAYMASTER, 0, 0, []byte{})), DEFAULT_BALANCE), types.Rip7560AccountAbstractionTx{ From eb7127c97d97b1cbe46353e6d9ff8cb3400c7951 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sat, 6 Jul 2024 15:49:03 +0300 Subject: [PATCH 5/5] pr reviews --- core/state_processor_rip7560.go | 26 ++++++++++++-------------- tests/rip7560/paymaster_test.go | 4 ++-- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/core/state_processor_rip7560.go b/core/state_processor_rip7560.go index 9e0333ed4d..599ee35682 100644 --- a/core/state_processor_rip7560.go +++ b/core/state_processor_rip7560.go @@ -36,15 +36,18 @@ func UnpackValidationData(validationData []byte) (authorizerMagic uint64, validU return } -func UnpackPaymasterValidationReturn(paymasterValidationReturn []byte) (validationData, context []byte) { +func UnpackPaymasterValidationReturn(paymasterValidationReturn []byte) (validationData, context []byte, err error) { if len(paymasterValidationReturn) < 96 { - return nil, nil + return nil, nil, errors.New("paymaster return data: too short") } validationData = paymasterValidationReturn[0:32] //2nd bytes32 is ignored (its an offset value) contextLen := new(big.Int).SetBytes(paymasterValidationReturn[64:96]) if uint64(len(paymasterValidationReturn)) < 96+contextLen.Uint64() { - return nil, nil + return nil, nil, errors.New("paymaster return data: unable to decode context") + } + if contextLen.Cmp(big.NewInt(PAYMASTER_MAX_CONTEXT_SIZE)) > 0 { + return nil, nil, errors.New("paymaster return data: context too large") } context = paymasterValidationReturn[96 : 96+contextLen.Uint64()] @@ -383,8 +386,7 @@ func prepareAccountValidationMessage(baseTx *types.Transaction, chainConfig *par func preparePaymasterValidationMessage(baseTx *types.Transaction, config *params.ChainConfig, signingHash common.Hash) (*Message, error) { tx := baseTx.Rip7560TransactionData() - paymasterAddress := tx.Paymaster - if paymasterAddress == nil { + if tx.Paymaster == nil { return nil, nil } jsondata := `[ @@ -400,7 +402,7 @@ func preparePaymasterValidationMessage(baseTx *types.Transaction, config *params } return &Message{ From: config.EntryPointAddress, - To: paymasterAddress, + To: tx.Paymaster, Value: big.NewInt(0), GasLimit: tx.PaymasterGas, GasPrice: tx.GasFeeCap, @@ -447,10 +449,9 @@ func preparePostOpMessage(vpr *ValidationPhaseResult, chainConfig *params.ChainC if err != nil { return nil, err } - var paymasterAddress = tx.Paymaster return &Message{ From: chainConfig.EntryPointAddress, - To: paymasterAddress, + To: tx.Paymaster, Value: big.NewInt(0), GasLimit: tx.PaymasterGas - executionResult.UsedGas, GasPrice: tx.GasFeeCap, @@ -482,17 +483,14 @@ func validatePaymasterReturnData(data []byte) (context []byte, validAfter, valid if len(data) < 32 { return nil, 0, 0, errors.New("invalid paymaster return data length") } - validationData, context := UnpackPaymasterValidationReturn(data) - if validationData == nil { - return nil, 0, 0, errors.New("invalid paymaster return data") + validationData, context, err := UnpackPaymasterValidationReturn(data) + if err != nil { + return nil, 0, 0, err } magicExpected, validUntil, validAfter := UnpackValidationData(validationData) if magicExpected != MAGIC_VALUE_PAYMASTER { return nil, 0, 0, errors.New("paymaster did not return correct MAGIC_VALUE") } - if len(context) > PAYMASTER_MAX_CONTEXT_SIZE { - return nil, 0, 0, errors.New("paymaster context too large") - } return context, validAfter, validUntil, nil } diff --git a/tests/rip7560/paymaster_test.go b/tests/rip7560/paymaster_test.go index e6a026d276..b169e0be23 100644 --- a/tests/rip7560/paymaster_test.go +++ b/tests/rip7560/paymaster_test.go @@ -49,7 +49,7 @@ func TestPaymasterValidationFailure_unparseable_return_value(t *testing.T) { PaymasterGas: 1000000000, GasFeeCap: big.NewInt(1000000000), Paymaster: &DEFAULT_PAYMASTER, - }, "invalid paymaster return data") + }, "paymaster return data: too short") } func TestPaymasterValidationFailure_wrong_magic(t *testing.T) { @@ -81,7 +81,7 @@ func TestPaymasterValidationFailure_contextTooLarge(t *testing.T) { PaymasterGas: 1000000000, GasFeeCap: big.NewInt(1000000000), Paymaster: &DEFAULT_PAYMASTER, - }, "paymaster context too large") + }, "paymaster return data: context too large") } func TestPaymasterValidationFailure_validAfter(t *testing.T) {