Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/clef, signer: support 1559 type transactions in clef #22966

Merged
merged 1 commit into from
Jun 9, 2021
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
19 changes: 12 additions & 7 deletions accounts/external/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,18 @@ func (api *ExternalSigner) SignTx(account accounts.Account, tx *types.Transactio
to = &t
}
args := &core.SendTxArgs{
Data: &data,
Nonce: hexutil.Uint64(tx.Nonce()),
Value: hexutil.Big(*tx.Value()),
Gas: hexutil.Uint64(tx.Gas()),
GasPrice: hexutil.Big(*tx.GasPrice()),
To: to,
From: common.NewMixedcaseAddress(account.Address),
Data: &data,
Nonce: hexutil.Uint64(tx.Nonce()),
Value: hexutil.Big(*tx.Value()),
Gas: hexutil.Uint64(tx.Gas()),
To: to,
From: common.NewMixedcaseAddress(account.Address),
}
if tx.GasFeeCap() != nil {
args.MaxFeePerGas = (*hexutil.Big)(tx.GasFeeCap())
args.MaxPriorityFeePerGas = (*hexutil.Big)(tx.GasTipCap())
} else {
args.GasPrice = (*hexutil.Big)(tx.GasPrice())
}
// We should request the default chain id that we're operating with
// (the chain we're executing on)
Expand Down
6 changes: 3 additions & 3 deletions cmd/clef/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ func testExternalUI(api *core.SignerAPI) {
Value: hexutil.Big(*big.NewInt(6)),
From: common.NewMixedcaseAddress(a),
To: &to,
GasPrice: hexutil.Big(*big.NewInt(5)),
GasPrice: (*hexutil.Big)(big.NewInt(5)),
Gas: 1000,
Input: nil,
}
Expand Down Expand Up @@ -1065,7 +1065,7 @@ func GenDoc(ctx *cli.Context) {
Value: hexutil.Big(*big.NewInt(6)),
From: common.NewMixedcaseAddress(a),
To: nil,
GasPrice: hexutil.Big(*big.NewInt(5)),
GasPrice: (*hexutil.Big)(big.NewInt(5)),
Gas: 1000,
Input: nil,
}})
Expand All @@ -1081,7 +1081,7 @@ func GenDoc(ctx *cli.Context) {
Value: hexutil.Big(*big.NewInt(6)),
From: common.NewMixedcaseAddress(a),
To: nil,
GasPrice: hexutil.Big(*big.NewInt(5)),
GasPrice: (*hexutil.Big)(big.NewInt(5)),
Gas: 1000,
Input: nil,
}})
Expand Down
16 changes: 16 additions & 0 deletions cmd/clef/testdata/sign_1559_missing_field_exp_fail.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"jsonrpc": "2.0",
"method": "account_signTransaction",
"params": [
{
"from": "0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"to": "0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"gas": "0x333",
"maxFeePerGas": "0x123",
"nonce": "0x0",
"value": "0x10",
"data": "0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"
}
],
"id": 67
}
16 changes: 16 additions & 0 deletions cmd/clef/testdata/sign_1559_missing_maxfeepergas_exp_fail.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"jsonrpc": "2.0",
"method": "account_signTransaction",
"params": [
{
"from": "0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"to": "0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"gas": "0x333",
"maxPriorityFeePerGas": "0x123",
"nonce": "0x0",
"value": "0x10",
"data": "0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"
}
],
"id": 67
}
17 changes: 17 additions & 0 deletions cmd/clef/testdata/sign_1559_tx.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"jsonrpc": "2.0",
"method": "account_signTransaction",
"params": [
{
"from": "0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"to": "0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"gas": "0x333",
"maxPriorityFeePerGas": "0x123",
"maxFeePerGas": "0x123",
"nonce": "0x0",
"value": "0x10",
"data": "0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"
}
],
"id": 67
}
17 changes: 17 additions & 0 deletions cmd/clef/testdata/sign_bad_checksum_exp_fail.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"jsonrpc": "2.0",
"method": "account_signTransaction",
"params": [
{
"from":"0x8a8eafb1cf62bfbeb1741769dae1a9dd47996192",
"to":"0x8a8eafb1cf62bfbeb1741769dae1a9dd47996192",
"gas": "0x333",
"gasPrice": "0x123",
"nonce": "0x0",
"value": "0x10",
"data":
"0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"
}
],
"id": 67
}
17 changes: 17 additions & 0 deletions cmd/clef/testdata/sign_normal_exp_ok.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
{
"jsonrpc": "2.0",
"method": "account_signTransaction",
"params": [
{
"from":"0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"to":"0x8A8eAFb1cf62BfBeb1741769DAE1a9dd47996192",
"gas": "0x333",
"gasPrice": "0x123",
"nonce": "0x0",
"value": "0x10",
"data":
"0x4401a6e40000000000000000000000000000000000000000000000000000000000000012"
}
],
"id": 67
}
6 changes: 6 additions & 0 deletions internal/ethapi/transaction_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,3 +265,9 @@ func (args *TransactionArgs) toTransaction() *types.Transaction {
}
return types.NewTx(data)
}

// ToTransaction converts the arguments to a transaction.
// This assumes that setDefaults has been called.
func (args *TransactionArgs) ToTransaction() *types.Transaction {
return args.toTransaction()
}
22 changes: 20 additions & 2 deletions signer/core/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,16 @@ func (api *SignerAPI) newAccount() (common.Address, error) {
// it also returns 'true' if the transaction was modified, to make it possible to configure the signer not to allow
// UI-modifications to requests
func logDiff(original *SignTxRequest, new *SignTxResponse) bool {
var intPtrModified = func(a, b *hexutil.Big) bool {
aBig := (*big.Int)(a)
bBig := (*big.Int)(b)
if aBig != nil && bBig != nil {
return aBig.Cmp(bBig) != 0
}
// One or both of them are nil
return a != b
}

modified := false
if f0, f1 := original.Transaction.From, new.Transaction.From; !reflect.DeepEqual(f0, f1) {
log.Info("Sender-account changed by UI", "was", f0, "is", f1)
Expand All @@ -469,9 +479,17 @@ func logDiff(original *SignTxRequest, new *SignTxResponse) bool {
modified = true
log.Info("Gas changed by UI", "was", g0, "is", g1)
}
if g0, g1 := big.Int(original.Transaction.GasPrice), big.Int(new.Transaction.GasPrice); g0.Cmp(&g1) != 0 {
if a, b := original.Transaction.GasPrice, new.Transaction.GasPrice; intPtrModified(a, b) {
log.Info("GasPrice changed by UI", "was", a, "is", b)
modified = true
}
if a, b := original.Transaction.MaxPriorityFeePerGas, new.Transaction.MaxPriorityFeePerGas; intPtrModified(a, b) {
log.Info("maxPriorityFeePerGas changed by UI", "was", a, "is", b)
modified = true
}
if a, b := original.Transaction.MaxFeePerGas, new.Transaction.MaxFeePerGas; intPtrModified(a, b) {
log.Info("maxFeePerGas changed by UI", "was", a, "is", b)
modified = true
log.Info("GasPrice changed by UI", "was", g0, "is", g1)
}
if v0, v1 := big.Int(original.Transaction.Value), big.Int(new.Transaction.Value); v0.Cmp(&v1) != 0 {
modified = true
Expand Down
2 changes: 1 addition & 1 deletion signer/core/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func mkTestTx(from common.MixedcaseAddress) core.SendTxArgs {
From: from,
To: &to,
Gas: gas,
GasPrice: gasPrice,
GasPrice: &gasPrice,
Value: value,
Data: &data,
Nonce: nonce}
Expand Down
13 changes: 9 additions & 4 deletions signer/core/cliui.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,15 @@ func (ui *CommandlineUI) ApproveTx(request *SignTxRequest) (SignTxResponse, erro
} else {
fmt.Printf("to: <contact creation>\n")
}
fmt.Printf("from: %v\n", request.Transaction.From.String())
fmt.Printf("value: %v wei\n", weival)
fmt.Printf("gas: %v (%v)\n", request.Transaction.Gas, uint64(request.Transaction.Gas))
fmt.Printf("gasprice: %v wei\n", request.Transaction.GasPrice.ToInt())
fmt.Printf("from: %v\n", request.Transaction.From.String())
fmt.Printf("value: %v wei\n", weival)
fmt.Printf("gas: %v (%v)\n", request.Transaction.Gas, uint64(request.Transaction.Gas))
if request.Transaction.MaxFeePerGas != nil {
fmt.Printf("maxFeePerGas: %v wei\n", request.Transaction.MaxFeePerGas.ToInt())
fmt.Printf("maxPriorityFeePerGas: %v wei\n", request.Transaction.MaxPriorityFeePerGas.ToInt())
} else {
fmt.Printf("gasprice: %v wei\n", request.Transaction.GasPrice.ToInt())
}
fmt.Printf("nonce: %v (%v)\n", request.Transaction.Nonce, uint64(request.Transaction.Nonce))
if chainId := request.Transaction.ChainID; chainId != nil {
fmt.Printf("chainid: %v\n", chainId)
Expand Down
3 changes: 2 additions & 1 deletion signer/core/gnosis_safe.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,12 @@ func (tx *GnosisSafeTx) ToTypedData() TypedData {
// ArgsForValidation returns a SendTxArgs struct, which can be used for the
// common validations, e.g. look up 4byte destinations
func (tx *GnosisSafeTx) ArgsForValidation() *SendTxArgs {
gp := hexutil.Big(tx.GasPrice)
args := &SendTxArgs{
From: tx.Safe,
To: &tx.To,
Gas: hexutil.Uint64(tx.SafeTxGas.Uint64()),
GasPrice: hexutil.Big(tx.GasPrice),
GasPrice: &gp,
Value: hexutil.Big(tx.Value),
Nonce: hexutil.Uint64(tx.Nonce.Uint64()),
Data: tx.Data,
Expand Down
67 changes: 29 additions & 38 deletions signer/core/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ package core
import (
"encoding/json"
"fmt"
"math/big"
"strings"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/internal/ethapi"
)

type ValidationInfo struct {
Expand Down Expand Up @@ -66,14 +66,21 @@ func (v *ValidationMessages) getWarnings() error {
}

// SendTxArgs represents the arguments to submit a transaction
// This struct is identical to ethapi.TransactionArgs, except for the usage of
// common.MixedcaseAddress in From and To
type SendTxArgs struct {
From common.MixedcaseAddress `json:"from"`
To *common.MixedcaseAddress `json:"to"`
Gas hexutil.Uint64 `json:"gas"`
GasPrice hexutil.Big `json:"gasPrice"`
Value hexutil.Big `json:"value"`
Nonce hexutil.Uint64 `json:"nonce"`
From common.MixedcaseAddress `json:"from"`
To *common.MixedcaseAddress `json:"to"`
Gas hexutil.Uint64 `json:"gas"`
GasPrice *hexutil.Big `json:"gasPrice"`
MaxFeePerGas *hexutil.Big `json:"maxFeePerGas"`
MaxPriorityFeePerGas *hexutil.Big `json:"maxPriorityFeePerGas"`
Value hexutil.Big `json:"value"`
Nonce hexutil.Uint64 `json:"nonce"`

// We accept "data" and "input" for backwards-compatibility reasons.
// "input" is the newer name and should be preferred by clients.
// Issue detail: https:/ethereum/go-ethereum/issues/15628
Data *hexutil.Bytes `json:"data"`
Input *hexutil.Bytes `json:"input,omitempty"`

Expand All @@ -91,38 +98,22 @@ func (args SendTxArgs) String() string {
}

func (args *SendTxArgs) toTransaction() *types.Transaction {
var input []byte
if args.Data != nil {
input = *args.Data
} else if args.Input != nil {
input = *args.Input
txArgs := ethapi.TransactionArgs{
Gas: &args.Gas,
GasPrice: args.GasPrice,
MaxFeePerGas: args.MaxFeePerGas,
MaxPriorityFeePerGas: args.MaxPriorityFeePerGas,
Value: &args.Value,
Nonce: &args.Nonce,
Data: args.Data,
Input: args.Input,
AccessList: args.AccessList,
ChainID: args.ChainID,
}
var to *common.Address
// Add the To-field, if specified
if args.To != nil {
_to := args.To.Address()
to = &_to
}
var data types.TxData
if args.AccessList == nil {
data = &types.LegacyTx{
To: to,
Nonce: uint64(args.Nonce),
Gas: uint64(args.Gas),
GasPrice: (*big.Int)(&args.GasPrice),
Value: (*big.Int)(&args.Value),
Data: input,
}
} else {
data = &types.AccessListTx{
To: to,
ChainID: (*big.Int)(args.ChainID),
Nonce: uint64(args.Nonce),
Gas: uint64(args.Gas),
GasPrice: (*big.Int)(&args.GasPrice),
Value: (*big.Int)(&args.Value),
Data: input,
AccessList: *args.AccessList,
}
to := args.To.Address()
txArgs.To = &to
}
return types.NewTx(data)
return txArgs.ToTransaction()
}
10 changes: 10 additions & 0 deletions signer/fourbyte/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,16 @@ func (db *Database) ValidateTransaction(selector *string, tx *core.SendTxArgs) (
if bytes.Equal(tx.To.Address().Bytes(), common.Address{}.Bytes()) {
messages.Crit("Transaction recipient is the zero address")
}
switch {
case tx.GasPrice == nil && tx.MaxFeePerGas == nil:
messages.Crit("Neither 'gasPrice' nor 'maxFeePerGas' specified.")
case tx.GasPrice == nil && tx.MaxPriorityFeePerGas == nil:
messages.Crit("Neither 'gasPrice' nor 'maxPriorityFeePerGas' specified.")
case tx.GasPrice != nil && tx.MaxFeePerGas != nil:
messages.Crit("Both 'gasPrice' and 'maxFeePerGas' specified.")
case tx.GasPrice != nil && tx.MaxPriorityFeePerGas != nil:
messages.Crit("Both 'gasPrice' and 'maxPriorityFeePerGas' specified.")
}
// Semantic fields validated, try to make heads or tails of the call data
db.ValidateCallData(selector, data, messages)
return messages, nil
Expand Down
2 changes: 1 addition & 1 deletion signer/fourbyte/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func dummyTxArgs(t txtestcase) *core.SendTxArgs {
To: to,
Value: value,
Nonce: n,
GasPrice: gasPrice,
GasPrice: &gasPrice,
Gas: gas,
Data: data,
Input: input,
Expand Down
2 changes: 1 addition & 1 deletion signer/rules/rules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func dummyTx(value hexutil.Big) *core.SignTxRequest {
To: to,
Value: value,
Nonce: n,
GasPrice: gasPrice,
GasPrice: &gasPrice,
Gas: gas,
},
Callinfo: []core.ValidationInfo{
Expand Down