From 6e2e44f32b8196f5e5eba66e4d45f5ff7c9c74ba Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 16:46:06 +0300 Subject: [PATCH 1/6] 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 6b7385d6a7a5b7323e44551b33aebb9622f4e732 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 14:17:25 +0300 Subject: [PATCH 2/6] 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 b0b490f350fa95920bbdfc389f6bea7c2f6a2681 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 20:49:00 +0300 Subject: [PATCH 3/6] 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 603d385c056ec82e7af293472184651f0e7b0a19 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Mon, 1 Jul 2024 22:11:30 +0300 Subject: [PATCH 4/6] 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 fcf91d4525753aff970e23350dfbc976e6ab8e2a Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Sat, 6 Jul 2024 15:49:03 +0300 Subject: [PATCH 5/6] 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) { From 65daa36d1faf12a4cd79be2e5eb770121a25b442 Mon Sep 17 00:00:00 2001 From: Dror Tirosh Date: Wed, 3 Jul 2024 18:50:20 +0300 Subject: [PATCH 6/6] test deployer flows --- core/state_processor_rip7560.go | 33 +++++++----- tests/rip7560/deployer_test.go | 88 +++++++++++++++++++++++++++++++ tests/rip7560/rip7560TestUtils.go | 36 +++++++++++-- 3 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 tests/rip7560/deployer_test.go diff --git a/core/state_processor_rip7560.go b/core/state_processor_rip7560.go index 599ee35682..13d4558ba2 100644 --- a/core/state_processor_rip7560.go +++ b/core/state_processor_rip7560.go @@ -177,8 +177,9 @@ func CheckNonceRip7560(tx *types.Rip7560AccountAbstractionTx, st *state.StateDB) func ApplyRip7560ValidationPhases(chainConfig *params.ChainConfig, bc ChainContext, author *common.Address, gp *GasPool, statedb *state.StateDB, header *types.Header, tx *types.Transaction, cfg vm.Config) (*ValidationPhaseResult, error) { blockContext := NewEVMBlockContext(header, bc, author) + sender := tx.Rip7560TransactionData().Sender txContext := vm.TxContext{ - Origin: *tx.Rip7560TransactionData().Sender, + Origin: *sender, GasPrice: tx.GasFeeCap(), } evm := vm.NewEVM(blockContext, txContext, statedb, chainConfig, cfg) @@ -186,16 +187,25 @@ func ApplyRip7560ValidationPhases(chainConfig *params.ChainConfig, bc ChainConte deployerMsg := prepareDeployerMessage(tx, chainConfig) var deploymentUsedGas uint64 if deployerMsg != nil { - resultDeployer, err := ApplyMessage(evm, deployerMsg, gp) + var err error + var resultDeployer *ExecutionResult + if statedb.GetCodeSize(*sender) != 0 { + err = errors.New("sender already deployed") + } else { + resultDeployer, err = ApplyMessage(evm, deployerMsg, gp) + } + if err == nil && resultDeployer != nil { + err = resultDeployer.Err + deploymentUsedGas = resultDeployer.UsedGas + } + if err == nil && statedb.GetCodeSize(*sender) == 0 { + err = errors.New("sender not deployed") + } if err != nil { - return nil, err + // TODO: bubble up the inner error message to the user, if possible + return nil, fmt.Errorf("account deployment failed: %v", err) } statedb.IntermediateRoot(true) - if resultDeployer.Failed() { - // TODO: bubble up the inner error message to the user, if possible - return nil, errors.New("account deployment failed - invalid transaction") - } - deploymentUsedGas = resultDeployer.UsedGas } /*** Account Validation Frame ***/ @@ -338,19 +348,18 @@ func ApplyRip7560ExecutionPhase(config *params.ChainConfig, vpr *ValidationPhase func prepareDeployerMessage(baseTx *types.Transaction, config *params.ChainConfig) *Message { tx := baseTx.Rip7560TransactionData() - if len(tx.DeployerData) < 20 { + if tx.Deployer == nil { return nil } - var deployerAddress common.Address = [20]byte(tx.DeployerData[0:20]) return &Message{ From: config.DeployerCallerAddress, - To: &deployerAddress, + To: tx.Deployer, Value: big.NewInt(0), GasLimit: tx.ValidationGas, GasPrice: tx.GasFeeCap, GasFeeCap: tx.GasFeeCap, GasTipCap: tx.GasTipCap, - Data: tx.DeployerData[20:], + Data: tx.DeployerData, AccessList: make(types.AccessList, 0), SkipAccountChecks: true, IsRip7560Frame: true, diff --git a/tests/rip7560/deployer_test.go b/tests/rip7560/deployer_test.go new file mode 100644 index 0000000000..e07ea49ef1 --- /dev/null +++ b/tests/rip7560/deployer_test.go @@ -0,0 +1,88 @@ +package rip7560 + +import ( + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "math/big" + "testing" +) + +var DEPLOYER = common.HexToAddress("0xddddddddddeeeeeeeeeeddddddddddeeeeeeeeee") + +func TestValidationFailure_deployerRevert(t *testing.T) { + handleTransaction(newTestContextBuilder(t). + withCode(DEFAULT_SENDER, []byte{}, DEFAULT_BALANCE). + withCode(DEPLOYER.Hex(), revertWithData([]byte{}), 0), + types.Rip7560AccountAbstractionTx{ + Deployer: &DEPLOYER, + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + }, "account deployment failed: execution reverted") +} + +func TestValidationFailure_deployerOOG(t *testing.T) { + handleTransaction(newTestContextBuilder(t). + withCode(DEFAULT_SENDER, []byte{}, DEFAULT_BALANCE). + withCode(DEPLOYER.Hex(), revertWithData([]byte{}), 0), + types.Rip7560AccountAbstractionTx{ + Deployer: &DEPLOYER, + ValidationGas: 1, + GasFeeCap: big.NewInt(1000000000), + }, "account deployment failed: out of gas") +} + +func TestValidationFailure_senderNotDeployed(t *testing.T) { + handleTransaction(newTestContextBuilder(t). + withCode(DEFAULT_SENDER, []byte{}, DEFAULT_BALANCE). + withCode(DEPLOYER.Hex(), returnWithData([]byte{}), 0), + types.Rip7560AccountAbstractionTx{ + Deployer: &DEPLOYER, + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + }, "account deployment failed: sender not deployed") +} + +func TestValidationFailure_senderAlreadyDeployed(t *testing.T) { + accountCode := revertWithData([]byte{}) + deployerCode := create2(accountCode) + sender := create2_addr(DEPLOYER, accountCode) + handleTransaction(newTestContextBuilder(t). + withCode(sender.Hex(), accountCode, DEFAULT_BALANCE). + withCode(DEPLOYER.Hex(), deployerCode, 0), + types.Rip7560AccountAbstractionTx{ + Sender: &sender, + Deployer: &DEPLOYER, + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + }, "account deployment failed: sender already deployed") +} + +func TestValidationFailure_senderReverts(t *testing.T) { + accountCode := revertWithData([]byte{}) + deployerCode := createCode(create2(accountCode), returnWithData([]byte{})) + sender := create2_addr(DEPLOYER, accountCode) + handleTransaction(newTestContextBuilder(t). + withCode(sender.Hex(), []byte{}, DEFAULT_BALANCE). + withCode(DEPLOYER.Hex(), deployerCode, 0), + types.Rip7560AccountAbstractionTx{ + Sender: &sender, + Deployer: &DEPLOYER, + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + }, "execution reverted") +} + +func TestValidation_deployer_ok(t *testing.T) { + accountCode := createAccountCode() + deployerCode := createCode(create2(accountCode), returnWithData([]byte{})) + sender := create2_addr(DEPLOYER, accountCode) + handleTransaction(newTestContextBuilder(t). + withCode(sender.Hex(), []byte{}, DEFAULT_BALANCE). + withCode(DEPLOYER.Hex(), deployerCode, 0), + types.Rip7560AccountAbstractionTx{ + Sender: &sender, + Deployer: &DEPLOYER, + ValidationGas: 1000000000, + GasFeeCap: big.NewInt(1000000000), + }, "ok") +} diff --git a/tests/rip7560/rip7560TestUtils.go b/tests/rip7560/rip7560TestUtils.go index 82c46650dd..5ebf610d00 100644 --- a/tests/rip7560/rip7560TestUtils.go +++ b/tests/rip7560/rip7560TestUtils.go @@ -8,6 +8,7 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/status-im/keycard-go/hexutils" "math/big" @@ -45,6 +46,31 @@ func newTestContextBuilder(t *testing.T) *testContextBuilder { } } +// return a contract code that will deploy the given code +func create2_contract(deployedCode []byte) []byte { + + return returnWithData(deployedCode) +} + +// return the generated address when deploying the given code +func create2_addr(deployer common.Address, deployedCode []byte) common.Address { + + contractCode := create2_contract(deployedCode) + data := createCode(0xff, deployer.Bytes(), common.Hash{}, crypto.Keccak256(contractCode)) + return common.BytesToAddress(crypto.Keccak256(data)) +} + +// generate code to call create2 +// note: parameter is the deployed code, not the full contract code +// always use zero value and zero salt. +func create2(deployedCode []byte) []byte { + contractCode := create2_contract(deployedCode) + return createCode( + copyToMemory(contractCode, 0), + push(0), push(len(contractCode)), push(0), push(0), vm.CREATE2, + ) +} + func (tb *testContextBuilder) build() *testContext { genesis := core.DeveloperGenesisBlock(10_000_000, &common.Address{}) genesis.Timestamp = 100 @@ -69,7 +95,6 @@ func (tt *testContextBuilder) withAccount(addr string, balance int64) *testConte tt.genesisAlloc[common.HexToAddress(addr)] = types.Account{Balance: big.NewInt(balance)} return tt } - func (tt *testContextBuilder) withCode(addr string, code []byte, balance int64) *testContextBuilder { if len(code) == 0 { tt.genesisAlloc[common.HexToAddress(addr)] = types.Account{ @@ -113,8 +138,9 @@ func copyToMemory(data []byte, offset uint) []byte { } if len(data) > 0 { - PUSHn := byte(int(vm.PUSH0) + len(data)) - ret = append(ret, createCode(PUSHn, data, vm.PUSH2, uint16(offset), vm.MSTORE)...) + //push data up, as EVM is big-endian + v := common.RightPadBytes(data, 32) + ret = append(ret, createCode(vm.PUSH32, v, vm.PUSH2, uint16(offset), vm.MSTORE)...) } return ret } @@ -160,6 +186,10 @@ func createCode(items ...interface{}) []byte { panic(fmt.Errorf("int defaults to int8 (byte). use int16, etc: %v", v)) } buffer.WriteByte(byte(v)) + case common.Hash: + buffer.Write(v.Bytes()) + case common.Address: + buffer.Write(v.Bytes()) default: // should be a compile-time error... panic(fmt.Errorf("unsupported type: %T", v))