From 29691ed0f6f78296c3cda0ac27b46637fbc3f6e9 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 22 Sep 2020 00:00:23 +0200 Subject: [PATCH 1/7] accounts, signer: implement gnosis safe support --- accounts/gnosis_safe.go | 143 +++++++++++++++++++++++++++++++++++ accounts/gnosis_safe_test.go | 62 +++++++++++++++ signer/core/api.go | 109 ++++++++++++++++++++++++++ signer/core/auditlog.go | 17 +++++ 4 files changed, 331 insertions(+) create mode 100644 accounts/gnosis_safe.go create mode 100644 accounts/gnosis_safe_test.go diff --git a/accounts/gnosis_safe.go b/accounts/gnosis_safe.go new file mode 100644 index 000000000000..6a76f542b859 --- /dev/null +++ b/accounts/gnosis_safe.go @@ -0,0 +1,143 @@ +package accounts + +import ( + "fmt" + "github.com/ethereum/go-ethereum/common/hexutil" + "math/big" + "strings" + + "github.com/ethereum/go-ethereum/accounts/abi" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/math" + "github.com/ethereum/go-ethereum/crypto" +) + +// gnosisSafe represents the functionality of +// https://github.com/gnosis/safe-contracts/blob/v1.1.1/contracts/GnosisSafe.sol +type gnosisSafe struct { + addr common.Address +} + +func (safe *gnosisSafe) domainSeparator() []byte { + // crypto.Keccak256Hash([]byte("EIP712Domain(address verifyingContract)")) + var domainSeparatorType = []byte("\x03Z\xff\x83\xd8i7\xd3[2\xe0O\r\xdco\xf4i)\x0e\xef/\x1bi-\x8a\x81\\\x89@MGI") + var ds []byte + ds = append(ds, domainSeparatorType...) + ds = append(ds, common.LeftPadBytes(safe.addr[:], 32)...) + return crypto.Keccak256(ds) +} + +// GnosisSafeSigningHash returns a tuple, txHash and signingHash. The latter is tied to the +// actual gnosis-safe used, whereas the former is transaction-specific. +// In order to confirm/sign a safe-tx, the latter needs to be signed by the +// keyholder +// +// The Gnosis safe uses a scheme where keyholders submit their signatures in a batch, and +// each keyholder thus signs the 'GnosisSafeTx' individually. The collected signatures are +// later submitted using only one single transaction. +// +// This method calculates the hash to sign based on a 'GnosisSafeTx' (the tx that is the end-goal +// of the entire signing round). +func GnosisSafeSigningHash(tx *GnosisSafeTx) (txHash, signingHash common.Hash, err error) { + // Calc the particular "safetx"-hash for the transaction + safeTxHash, err := tx.hash() + if err != nil { + return common.Hash{}, common.Hash{}, err + } + safe := gnosisSafe{tx.Safe.Address()} + + // Put together the final preimage + msg := []byte{0x19, 0x01} + // Calc the safe-specific domain separator (depends on address) + msg = append(msg, safe.domainSeparator()...) + msg = append(msg, safeTxHash[:]...) + // Final hash for signing + signingHash = crypto.Keccak256Hash(msg) + + if tx.InputExpHash != (common.Hash{}) && tx.InputExpHash != signingHash { + return common.Hash{}, + common.Hash{}, + fmt.Errorf("expected hash differs from calculated, input-expectation was %x, got %x", + tx.InputExpHash, signingHash) + } + return safeTxHash, signingHash, nil +} + +type GnosisSafeTx struct { + Safe common.MixedcaseAddress `json:"safe"` + To common.MixedcaseAddress `json:"to"` + Value math.HexOrDecimal256 `json:"value"` + GasPrice math.HexOrDecimal256 `json:"gasPrice"` + Data *hexutil.Bytes `json:"data"` + Operation uint8 `json:"operation"` + GasToken common.Address `json:"gasToken"` + RefundReceiver common.Address `json:"refundReceiver"` + BaseGas big.Int `json:"baseGas"` + SafeTxGas big.Int `json:"safeTxGas"` + Nonce big.Int `json:"nonce"` + InputExpHash common.Hash `json:"safeTxHash"` +} + +/** +hash implements the following solidity construct: + + function encodeTransactionData( + address to, + uint256 value, + bytes memory data, + Enum.Operation operation, + uint256 safeTxGas, + uint256 baseGas, + uint256 gasPrice, + address gasToken, + address refundReceiver, + uint256 nonce) + +bytes32 safeTxHash = keccak256( + abi.encode(SAFE_TX_TYPEHASH, to, value, keccak256(data), operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, _nonce) +); +*/ +func (tx *GnosisSafeTx) hash() (common.Hash, error) { + if tx.Operation != 0 { + return common.Hash{}, fmt.Errorf("Signing type %d not implemented", tx.Operation) + } + // crypto.Keccak256Hash([]byte("SafeTx(address to,uint256 value,bytes data,uint8 operation,uint256 safeTxGas,uint256 baseGas,uint256 gasPrice,address gasToken,address refundReceiver,uint256 nonce)")) + var safeTxHashType = common.Hash{0xbb, 0x83, 0x10, 0xd4, 0x86, 0x36, 0x8d, 0xb6, 0xbd, 0x6f, 0x84, 0x94, 0x02, 0xfd, 0xd7, 0x3a, 0xd5, 0x3d, 0x31, 0x6b, 0x5a, 0x4b, 0x26, 0x44, 0xad, 0x6e, 0xfe, 0x0f, 0x94, 0x12, 0x86, 0xd8} + // We use the ABI below to aid packing the preimage + var txPackABI = `[{ + "name": "encodeTransactionData", + "inputs": [ + {"name": "hashType","type": "bytes32"}, + {"name": "to","type": "address"}, + {"name": "value","type": "uint256"}, + {"name": "datahash","type": "bytes32"}, + {"name": "operation","type": "uint8"}, + {"name": "safeTxGas","type": "uint256"}, + {"name": "baseGas","type": "uint256"}, + {"name": "gasPrice","type": "uint256"}, + {"name": "gasToken","type": "address"}, + {"name": "refundReceiver","type": "address"}, + {"name": "_nonce","type": "uint256"} + ], + "type": "function" + }]` + abispec, err := abi.JSON(strings.NewReader(txPackABI)) + if err != nil { + return (common.Hash{}), err + } + val := big.Int(tx.Value) + gasPrice := big.Int(tx.GasPrice) + // Pack the fields + var data []byte + if tx.Data != nil{ + data = []byte(*tx.Data) + } + packed, err := abispec.Methods["encodeTransactionData"].Inputs.Pack( + safeTxHashType, tx.To.Address(), &val, crypto.Keccak256Hash(data), tx.Operation, + &tx.SafeTxGas, &tx.BaseGas, &gasPrice, tx.GasToken, tx.RefundReceiver, &tx.Nonce, + ) + if err != nil { + return common.Hash{}, err + } + return crypto.Keccak256Hash(packed), nil +} diff --git a/accounts/gnosis_safe_test.go b/accounts/gnosis_safe_test.go new file mode 100644 index 000000000000..da566160477d --- /dev/null +++ b/accounts/gnosis_safe_test.go @@ -0,0 +1,62 @@ +package accounts + +import ( + "encoding/json" + "testing" + + "github.com/ethereum/go-ethereum/common" +) + +func TestGnosisSafe(t *testing.T) { + var txjson = `{ + "safe": "0x25a6c4BBd32B2424A9c99aEB0584Ad12045382B3", + "to": "0x9eE457023bB3De16D51A003a247BaEaD7fce313D", + "value": "20000000000000000", + "data": null, + "operation": 0, + "gasToken": "0x0000000000000000000000000000000000000000", + "safeTxGas": 27845, + "baseGas": 0, + "gasPrice": "0", + "refundReceiver": "0x0000000000000000000000000000000000000000", + "nonce": 3, + "executionDate": null, + "submissionDate": "2020-09-15T21:59:23.815748Z", + "modified": "2020-09-15T21:59:23.815748Z", + "blockNumber": null, + "transactionHash": null, + "safeTxHash": "0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f", + "executor": null, + "isExecuted": false, + "isSuccessful": null, + "ethGasPrice": null, + "gasUsed": null, + "fee": null, + "origin": null, + "dataDecoded": null, + "confirmationsRequired": null, + "confirmations": [ + { + "owner": "0xAd2e180019FCa9e55CADe76E4487F126Fd08DA34", + "submissionDate": "2020-09-15T21:59:28.281243Z", + "transactionHash": null, + "confirmationType": "CONFIRMATION", + "signature": "0x5e562065a0cb15d766dac0cd49eb6d196a41183af302c4ecad45f1a81958d7797753f04424a9b0aa1cb0448e4ec8e189540fbcdda7530ef9b9d95dfc2d36cb521b", + "signatureType": "EOA" + } + ], + "signatures": null + }` + var tx GnosisSafeTx + if err := json.Unmarshal([]byte(txjson), &tx); err != nil { + t.Fatal(err) + } + _, signingHash, err := GnosisSafeSigningHash(&tx) + if err != nil { + t.Fatal(err) + } + expHash := common.HexToHash("0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f") + if expHash != signingHash { + t.Fatalf("Got %x, exp %x", signingHash, expHash) + } +} diff --git a/signer/core/api.go b/signer/core/api.go index 3817345c8f1a..ef209ee14fa1 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -62,6 +62,8 @@ type ExternalAPI interface { EcRecover(ctx context.Context, data hexutil.Bytes, sig hexutil.Bytes) (common.Address, error) // Version info about the APIs Version(ctx context.Context) (string, error) + // SignGnosisSafeTransaction signs/confirms a gnosis-safe multisig transaction + SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) } // UIClientAPI specifies what method a UI needs to implement to be able to be used as a @@ -234,6 +236,7 @@ type ( Address common.MixedcaseAddress `json:"address"` Rawdata []byte `json:"raw_data"` Messages []*NameValueType `json:"messages"` + Callinfo []ValidationInfo `json:"call_info"` Hash hexutil.Bytes `json:"hash"` Meta Metadata `json:"meta"` } @@ -581,6 +584,112 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth } +type GnosisSigningResult struct { + Signature hexutil.Bytes `json:"signature"` + HashToSign common.Hash `json:"hashToSign"` + SafeTxHash common.Hash `json:"safeTxHash"` +} + +func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) { + // Do the usual validations, but on the last-stage transaction + args := &SendTxArgs{ + From: gnosisTx.Safe, + To: &gnosisTx.To, + Gas: hexutil.Uint64(gnosisTx.SafeTxGas.Uint64()), + GasPrice: hexutil.Big(gnosisTx.GasPrice), + Value: hexutil.Big(gnosisTx.Value), + Nonce: hexutil.Uint64(gnosisTx.Nonce.Uint64()), + Data: gnosisTx.Data, + Input: nil, + } + msgs, err := api.validator.ValidateTransaction(methodSelector, args) + if err != nil { + return nil, err + } + // If we are in 'rejectMode', then reject rather than show the user warnings + if api.rejectMode { + if err := msgs.getWarnings(); err != nil { + return nil, err + } + } + + safeTxHash, hashToSign, err := accounts.GnosisSafeSigningHash(&gnosisTx) + if err != nil { + return nil, err + } + weival := big.Int(gnosisTx.Value) + var messages = []*NameValueType{ + &NameValueType{ + Name: "This is a signing-request for a Gnosis multisig transaction", + Value: "info", Typ: "", + }, + &NameValueType{ + Name: "Multisig address", + Value: gnosisTx.Safe.Address().String(), Typ: "address", + }, + &NameValueType{ + Name: " Value", + Value: fmt.Sprintf("%v wei", &weival), Typ: "uint256", + }, + &NameValueType{ + Name: " Gas price", + Value: gnosisTx.SafeTxGas.String(), Typ: "uint256", + }, + &NameValueType{ + Name: " Nonce", + Value: gnosisTx.Nonce.String(), Typ: "uint256", + }, + } + if gnosisTx.Data != nil { + messages = append(messages, &NameValueType{ + Name: " Data", + Value: gnosisTx.Data, Typ: "hexdata", + }) + } + + req := SignDataRequest{ + ContentType: "application/gnosis", + Address: signerAddress, + //Rawdata: hashToSign[:], + Messages: messages, + Callinfo: msgs.Messages, + Hash: hashToSign.Bytes(), + Meta: MetadataFromContext(ctx), + } + // Process approval + result, err := api.UI.ApproveSignData(&req) + if err != nil { + return nil, err + } + if !result.Approved { + return nil, ErrRequestDenied + } + account := accounts.Account{Address: signerAddress.Address()} + wallet, err := api.am.Find(account) + if err != nil { + return nil, err + } + pw, err := api.lookupOrQueryPassword(account.Address, + "Password for signing", + fmt.Sprintf("Please enter password for signing data with account %s", account.Address.Hex())) + if err != nil { + return nil, err + } + // Sign the data with the wallet + signature, err := wallet.SignDataWithPassphrase(account, pw, req.ContentType, hashToSign[:]) + if err != nil { + return nil, err + } + if signature[64] < 2 { + signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper + } + return &GnosisSigningResult{ + signature, + hashToSign, + safeTxHash, + }, nil +} + // Returns the external api version. This method does not require user acceptance. Available methods are // available via enumeration anyway, and this info does not contain user-specific data func (api *SignerAPI) Version(ctx context.Context) (string, error) { diff --git a/signer/core/auditlog.go b/signer/core/auditlog.go index 1092e7a92340..a8cadf1a8d5c 100644 --- a/signer/core/auditlog.go +++ b/signer/core/auditlog.go @@ -18,6 +18,7 @@ package core import ( "context" + "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -68,6 +69,22 @@ func (l *AuditLogger) SignData(ctx context.Context, contentType string, addr com return b, e } +func (l *AuditLogger) SignGnosisTx(ctx context.Context, addr common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) { + sel := "" + if methodSelector != nil { + sel = *methodSelector + } + l.log.Info("SignGnosisTx", "type", "request", "metadata", MetadataFromContext(ctx).String(), + "addr", addr.String(), "data", gnosisTx, "selector", sel) + res, e := l.api.SignGnosisTx(ctx, addr, gnosisTx, methodSelector) + if res != nil { + l.log.Info("SignGnosisTx", "type", "response", "data", common.Bytes2Hex(res.Signature), "error", e) + } else { + l.log.Info("SignTransaction", "type", "response", "data", res, "error", e) + } + return res, e +} + func (l *AuditLogger) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, data TypedData) (hexutil.Bytes, error) { l.log.Info("SignTypedData", "type", "request", "metadata", MetadataFromContext(ctx).String(), "addr", addr.String(), "data", data) From 5a9ac0538dfd4569decb7014683f9d703ee9ec69 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 22 Sep 2020 09:36:40 +0200 Subject: [PATCH 2/7] common/math: add type for marshalling big to dec --- common/math/big.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/common/math/big.go b/common/math/big.go index 17a57df9dc78..60f63f62f5de 100644 --- a/common/math/big.go +++ b/common/math/big.go @@ -67,6 +67,35 @@ func (i *HexOrDecimal256) MarshalText() ([]byte, error) { return []byte(fmt.Sprintf("%#x", (*big.Int)(i))), nil } +// Decimal256 unmarshals big.Int as a decimal string. When unmarshalling, +// it however accepts either "0x"-prefixed (hex encoded) or non-prefixed (decimal) +type Decimal256 big.Int + +// NewHexOrDecimal256 creates a new Decimal256 +func NewDecimal256(x int64) *Decimal256 { + b := big.NewInt(x) + d := Decimal256(*b) + return &d +} + +// UnmarshalText implements encoding.TextUnmarshaler. +func (i *Decimal256) UnmarshalText(input []byte) error { + bigint, ok := ParseBig256(string(input)) + if !ok { + return fmt.Errorf("invalid hex or decimal integer %q", input) + } + *i = Decimal256(*bigint) + return nil +} + +// MarshalText implements encoding.TextMarshaler. +func (i *Decimal256) MarshalText() ([]byte, error) { + if i == nil { + return []byte("0"), nil + } + return []byte(fmt.Sprintf("%#d", (*big.Int)(i))), nil +} + // ParseBig256 parses s as a 256 bit integer in decimal or hexadecimal syntax. // Leading zeros are accepted. The empty string parses as zero. func ParseBig256(s string) (*big.Int, bool) { From 22857d2660cee8cd06edc0bde03a262fe6251ae1 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 22 Sep 2020 09:39:52 +0200 Subject: [PATCH 3/7] accounts, signer: properly sign gnosis requests --- accounts/gnosis_safe.go | 15 +++++----- accounts/gnosis_safe_test.go | 2 +- signer/core/api.go | 57 ++++++++++++++++++++++++++++-------- 3 files changed, 52 insertions(+), 22 deletions(-) diff --git a/accounts/gnosis_safe.go b/accounts/gnosis_safe.go index 6a76f542b859..e5f15e5ee667 100644 --- a/accounts/gnosis_safe.go +++ b/accounts/gnosis_safe.go @@ -38,11 +38,11 @@ func (safe *gnosisSafe) domainSeparator() []byte { // // This method calculates the hash to sign based on a 'GnosisSafeTx' (the tx that is the end-goal // of the entire signing round). -func GnosisSafeSigningHash(tx *GnosisSafeTx) (txHash, signingHash common.Hash, err error) { +func GnosisSafeSigningHash(tx *GnosisSafeTx) (signingHash common.Hash, preimage []byte, err error) { // Calc the particular "safetx"-hash for the transaction safeTxHash, err := tx.hash() if err != nil { - return common.Hash{}, common.Hash{}, err + return common.Hash{}, nil, err } safe := gnosisSafe{tx.Safe.Address()} @@ -55,19 +55,18 @@ func GnosisSafeSigningHash(tx *GnosisSafeTx) (txHash, signingHash common.Hash, e signingHash = crypto.Keccak256Hash(msg) if tx.InputExpHash != (common.Hash{}) && tx.InputExpHash != signingHash { - return common.Hash{}, - common.Hash{}, + return common.Hash{}, nil, fmt.Errorf("expected hash differs from calculated, input-expectation was %x, got %x", tx.InputExpHash, signingHash) } - return safeTxHash, signingHash, nil + return signingHash, msg, nil } type GnosisSafeTx struct { Safe common.MixedcaseAddress `json:"safe"` To common.MixedcaseAddress `json:"to"` - Value math.HexOrDecimal256 `json:"value"` - GasPrice math.HexOrDecimal256 `json:"gasPrice"` + Value math.Decimal256 `json:"value"` + GasPrice math.Decimal256 `json:"gasPrice"` Data *hexutil.Bytes `json:"data"` Operation uint8 `json:"operation"` GasToken common.Address `json:"gasToken"` @@ -129,7 +128,7 @@ func (tx *GnosisSafeTx) hash() (common.Hash, error) { gasPrice := big.Int(tx.GasPrice) // Pack the fields var data []byte - if tx.Data != nil{ + if tx.Data != nil { data = []byte(*tx.Data) } packed, err := abispec.Methods["encodeTransactionData"].Inputs.Pack( diff --git a/accounts/gnosis_safe_test.go b/accounts/gnosis_safe_test.go index da566160477d..8d67e6d53ccf 100644 --- a/accounts/gnosis_safe_test.go +++ b/accounts/gnosis_safe_test.go @@ -51,7 +51,7 @@ func TestGnosisSafe(t *testing.T) { if err := json.Unmarshal([]byte(txjson), &tx); err != nil { t.Fatal(err) } - _, signingHash, err := GnosisSafeSigningHash(&tx) + signingHash, _, err := GnosisSafeSigningHash(&tx) if err != nil { t.Fatal(err) } diff --git a/signer/core/api.go b/signer/core/api.go index ef209ee14fa1..ff465aca5a61 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -31,6 +31,7 @@ import ( "github.com/ethereum/go-ethereum/accounts/usbwallet" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" @@ -584,10 +585,24 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth } +// GnosisSigningResult conforms to the API required by the Gnosis Safe tx relay service. +// See 'SafeMultisigTransaction' on https://safe-transaction.mainnet.gnosis.io/ type GnosisSigningResult struct { - Signature hexutil.Bytes `json:"signature"` - HashToSign common.Hash `json:"hashToSign"` - SafeTxHash common.Hash `json:"safeTxHash"` + Signature hexutil.Bytes `json:"signature"` + SafeTxHash common.Hash `json:"contractTransactionHash"` + Sender common.MixedcaseAddress `json:"sender"` + // Other fields that are required for the tx relay service + Safe common.MixedcaseAddress `json:"safe"` + To common.MixedcaseAddress `json:"to"` + Value math.Decimal256 `json:"value"` + GasPrice math.Decimal256 `json:"gasPrice"` + Data *hexutil.Bytes `json:"data"` + Operation uint8 `json:"operation"` + GasToken common.Address `json:"gasToken"` + RefundReceiver common.Address `json:"refundReceiver"` + BaseGas big.Int `json:"baseGas"` + SafeTxGas big.Int `json:"safeTxGas"` + Nonce big.Int `json:"nonce"` } func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) { @@ -613,7 +628,7 @@ func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.Mix } } - safeTxHash, hashToSign, err := accounts.GnosisSafeSigningHash(&gnosisTx) + safeTxHash, preImage, err := accounts.GnosisSafeSigningHash(&gnosisTx) if err != nil { return nil, err } @@ -631,6 +646,10 @@ func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.Mix Name: " Value", Value: fmt.Sprintf("%v wei", &weival), Typ: "uint256", }, + &NameValueType{ + Name: " To", + Value: gnosisTx.To.String(), Typ: "address", + }, &NameValueType{ Name: " Gas price", Value: gnosisTx.SafeTxGas.String(), Typ: "uint256", @@ -650,11 +669,11 @@ func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.Mix req := SignDataRequest{ ContentType: "application/gnosis", Address: signerAddress, - //Rawdata: hashToSign[:], - Messages: messages, - Callinfo: msgs.Messages, - Hash: hashToSign.Bytes(), - Meta: MetadataFromContext(ctx), + Rawdata: preImage, + Messages: messages, + Callinfo: msgs.Messages, + Hash: safeTxHash.Bytes(), + Meta: MetadataFromContext(ctx), } // Process approval result, err := api.UI.ApproveSignData(&req) @@ -676,17 +695,29 @@ func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.Mix return nil, err } // Sign the data with the wallet - signature, err := wallet.SignDataWithPassphrase(account, pw, req.ContentType, hashToSign[:]) + signature, err := wallet.SignDataWithPassphrase(account, pw, req.ContentType, req.Rawdata) if err != nil { return nil, err } if signature[64] < 2 { signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper } + checkSummedSender, _ := common.NewMixedcaseAddressFromString(account.Address.Hex()) return &GnosisSigningResult{ - signature, - hashToSign, - safeTxHash, + Signature: signature, + SafeTxHash: safeTxHash, + Sender: *checkSummedSender, // Must be checksummed + Safe: gnosisTx.Safe, + To: gnosisTx.To, + Value: gnosisTx.Value, // must be decimal string + GasPrice: gnosisTx.GasPrice, // must be decimal string + Data: gnosisTx.Data, + Operation: gnosisTx.Operation, + GasToken: gnosisTx.GasToken, + RefundReceiver: gnosisTx.RefundReceiver, + BaseGas: gnosisTx.BaseGas, + SafeTxGas: gnosisTx.SafeTxGas, + Nonce: gnosisTx.Nonce, }, nil } From f72e4ccfe0e499c48e1b3fdb563293b0d06ace6f Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 22 Sep 2020 15:18:23 +0200 Subject: [PATCH 4/7] signer, clef: implement account_signGnosisTx --- accounts/gnosis_safe.go | 142 -------------------------------- accounts/gnosis_safe_test.go | 62 -------------- cmd/clef/extapi_changelog.md | 58 +++++++++++++ common/math/big.go | 9 +- signer/core/api.go | 136 +++--------------------------- signer/core/auditlog.go | 3 +- signer/core/gnosis_safe.go | 91 ++++++++++++++++++++ signer/core/signed_data.go | 34 +++++--- signer/core/signed_data_test.go | 117 ++++++++++++++++++++++++++ 9 files changed, 309 insertions(+), 343 deletions(-) delete mode 100644 accounts/gnosis_safe.go delete mode 100644 accounts/gnosis_safe_test.go create mode 100644 signer/core/gnosis_safe.go diff --git a/accounts/gnosis_safe.go b/accounts/gnosis_safe.go deleted file mode 100644 index e5f15e5ee667..000000000000 --- a/accounts/gnosis_safe.go +++ /dev/null @@ -1,142 +0,0 @@ -package accounts - -import ( - "fmt" - "github.com/ethereum/go-ethereum/common/hexutil" - "math/big" - "strings" - - "github.com/ethereum/go-ethereum/accounts/abi" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/math" - "github.com/ethereum/go-ethereum/crypto" -) - -// gnosisSafe represents the functionality of -// https://github.com/gnosis/safe-contracts/blob/v1.1.1/contracts/GnosisSafe.sol -type gnosisSafe struct { - addr common.Address -} - -func (safe *gnosisSafe) domainSeparator() []byte { - // crypto.Keccak256Hash([]byte("EIP712Domain(address verifyingContract)")) - var domainSeparatorType = []byte("\x03Z\xff\x83\xd8i7\xd3[2\xe0O\r\xdco\xf4i)\x0e\xef/\x1bi-\x8a\x81\\\x89@MGI") - var ds []byte - ds = append(ds, domainSeparatorType...) - ds = append(ds, common.LeftPadBytes(safe.addr[:], 32)...) - return crypto.Keccak256(ds) -} - -// GnosisSafeSigningHash returns a tuple, txHash and signingHash. The latter is tied to the -// actual gnosis-safe used, whereas the former is transaction-specific. -// In order to confirm/sign a safe-tx, the latter needs to be signed by the -// keyholder -// -// The Gnosis safe uses a scheme where keyholders submit their signatures in a batch, and -// each keyholder thus signs the 'GnosisSafeTx' individually. The collected signatures are -// later submitted using only one single transaction. -// -// This method calculates the hash to sign based on a 'GnosisSafeTx' (the tx that is the end-goal -// of the entire signing round). -func GnosisSafeSigningHash(tx *GnosisSafeTx) (signingHash common.Hash, preimage []byte, err error) { - // Calc the particular "safetx"-hash for the transaction - safeTxHash, err := tx.hash() - if err != nil { - return common.Hash{}, nil, err - } - safe := gnosisSafe{tx.Safe.Address()} - - // Put together the final preimage - msg := []byte{0x19, 0x01} - // Calc the safe-specific domain separator (depends on address) - msg = append(msg, safe.domainSeparator()...) - msg = append(msg, safeTxHash[:]...) - // Final hash for signing - signingHash = crypto.Keccak256Hash(msg) - - if tx.InputExpHash != (common.Hash{}) && tx.InputExpHash != signingHash { - return common.Hash{}, nil, - fmt.Errorf("expected hash differs from calculated, input-expectation was %x, got %x", - tx.InputExpHash, signingHash) - } - return signingHash, msg, nil -} - -type GnosisSafeTx struct { - Safe common.MixedcaseAddress `json:"safe"` - To common.MixedcaseAddress `json:"to"` - Value math.Decimal256 `json:"value"` - GasPrice math.Decimal256 `json:"gasPrice"` - Data *hexutil.Bytes `json:"data"` - Operation uint8 `json:"operation"` - GasToken common.Address `json:"gasToken"` - RefundReceiver common.Address `json:"refundReceiver"` - BaseGas big.Int `json:"baseGas"` - SafeTxGas big.Int `json:"safeTxGas"` - Nonce big.Int `json:"nonce"` - InputExpHash common.Hash `json:"safeTxHash"` -} - -/** -hash implements the following solidity construct: - - function encodeTransactionData( - address to, - uint256 value, - bytes memory data, - Enum.Operation operation, - uint256 safeTxGas, - uint256 baseGas, - uint256 gasPrice, - address gasToken, - address refundReceiver, - uint256 nonce) - -bytes32 safeTxHash = keccak256( - abi.encode(SAFE_TX_TYPEHASH, to, value, keccak256(data), operation, safeTxGas, baseGas, gasPrice, gasToken, refundReceiver, _nonce) -); -*/ -func (tx *GnosisSafeTx) hash() (common.Hash, error) { - if tx.Operation != 0 { - return common.Hash{}, fmt.Errorf("Signing type %d not implemented", tx.Operation) - } - // crypto.Keccak256Hash([]byte("SafeTx(address to,uint256 value,bytes data,uint8 operation,uint256 safeTxGas,uint256 baseGas,uint256 gasPrice,address gasToken,address refundReceiver,uint256 nonce)")) - var safeTxHashType = common.Hash{0xbb, 0x83, 0x10, 0xd4, 0x86, 0x36, 0x8d, 0xb6, 0xbd, 0x6f, 0x84, 0x94, 0x02, 0xfd, 0xd7, 0x3a, 0xd5, 0x3d, 0x31, 0x6b, 0x5a, 0x4b, 0x26, 0x44, 0xad, 0x6e, 0xfe, 0x0f, 0x94, 0x12, 0x86, 0xd8} - // We use the ABI below to aid packing the preimage - var txPackABI = `[{ - "name": "encodeTransactionData", - "inputs": [ - {"name": "hashType","type": "bytes32"}, - {"name": "to","type": "address"}, - {"name": "value","type": "uint256"}, - {"name": "datahash","type": "bytes32"}, - {"name": "operation","type": "uint8"}, - {"name": "safeTxGas","type": "uint256"}, - {"name": "baseGas","type": "uint256"}, - {"name": "gasPrice","type": "uint256"}, - {"name": "gasToken","type": "address"}, - {"name": "refundReceiver","type": "address"}, - {"name": "_nonce","type": "uint256"} - ], - "type": "function" - }]` - abispec, err := abi.JSON(strings.NewReader(txPackABI)) - if err != nil { - return (common.Hash{}), err - } - val := big.Int(tx.Value) - gasPrice := big.Int(tx.GasPrice) - // Pack the fields - var data []byte - if tx.Data != nil { - data = []byte(*tx.Data) - } - packed, err := abispec.Methods["encodeTransactionData"].Inputs.Pack( - safeTxHashType, tx.To.Address(), &val, crypto.Keccak256Hash(data), tx.Operation, - &tx.SafeTxGas, &tx.BaseGas, &gasPrice, tx.GasToken, tx.RefundReceiver, &tx.Nonce, - ) - if err != nil { - return common.Hash{}, err - } - return crypto.Keccak256Hash(packed), nil -} diff --git a/accounts/gnosis_safe_test.go b/accounts/gnosis_safe_test.go deleted file mode 100644 index 8d67e6d53ccf..000000000000 --- a/accounts/gnosis_safe_test.go +++ /dev/null @@ -1,62 +0,0 @@ -package accounts - -import ( - "encoding/json" - "testing" - - "github.com/ethereum/go-ethereum/common" -) - -func TestGnosisSafe(t *testing.T) { - var txjson = `{ - "safe": "0x25a6c4BBd32B2424A9c99aEB0584Ad12045382B3", - "to": "0x9eE457023bB3De16D51A003a247BaEaD7fce313D", - "value": "20000000000000000", - "data": null, - "operation": 0, - "gasToken": "0x0000000000000000000000000000000000000000", - "safeTxGas": 27845, - "baseGas": 0, - "gasPrice": "0", - "refundReceiver": "0x0000000000000000000000000000000000000000", - "nonce": 3, - "executionDate": null, - "submissionDate": "2020-09-15T21:59:23.815748Z", - "modified": "2020-09-15T21:59:23.815748Z", - "blockNumber": null, - "transactionHash": null, - "safeTxHash": "0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f", - "executor": null, - "isExecuted": false, - "isSuccessful": null, - "ethGasPrice": null, - "gasUsed": null, - "fee": null, - "origin": null, - "dataDecoded": null, - "confirmationsRequired": null, - "confirmations": [ - { - "owner": "0xAd2e180019FCa9e55CADe76E4487F126Fd08DA34", - "submissionDate": "2020-09-15T21:59:28.281243Z", - "transactionHash": null, - "confirmationType": "CONFIRMATION", - "signature": "0x5e562065a0cb15d766dac0cd49eb6d196a41183af302c4ecad45f1a81958d7797753f04424a9b0aa1cb0448e4ec8e189540fbcdda7530ef9b9d95dfc2d36cb521b", - "signatureType": "EOA" - } - ], - "signatures": null - }` - var tx GnosisSafeTx - if err := json.Unmarshal([]byte(txjson), &tx); err != nil { - t.Fatal(err) - } - signingHash, _, err := GnosisSafeSigningHash(&tx) - if err != nil { - t.Fatal(err) - } - expHash := common.HexToHash("0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f") - if expHash != signingHash { - t.Fatalf("Got %x, exp %x", signingHash, expHash) - } -} diff --git a/cmd/clef/extapi_changelog.md b/cmd/clef/extapi_changelog.md index dbc302631bc1..6bb068430716 100644 --- a/cmd/clef/extapi_changelog.md +++ b/cmd/clef/extapi_changelog.md @@ -10,6 +10,64 @@ TL;DR: Given a version number MAJOR.MINOR.PATCH, increment the: Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format. +### 6.1.0 + +The API-method `account_signGnosisTx` was added. This method takes two parameters, +`[address, safeTx]`. The latter, `safeTx`, can be copy-pasted from the gnosis relay. For example: + +``` +{ + "jsonrpc": "2.0", + "method": "account_signGnosisTx", + "params": ["0xfd1c4226bfD1c436672092F4eCbfC270145b7256", + { + "safe": "0x25a6c4BBd32B2424A9c99aEB0584Ad12045382B3", + "to": "0xB372a646f7F05Cc1785018dBDA7EBc734a2A20E2", + "value": "20000000000000000", + "data": null, + "operation": 0, + "gasToken": "0x0000000000000000000000000000000000000000", + "safeTxGas": 27845, + "baseGas": 0, + "gasPrice": "0", + "refundReceiver": "0x0000000000000000000000000000000000000000", + "nonce": 2, + "executionDate": null, + "submissionDate": "2020-09-15T21:54:49.617634Z", + "modified": "2020-09-15T21:54:49.617634Z", + "blockNumber": null, + "transactionHash": null, + "safeTxHash": "0x2edfbd5bc113ff18c0631595db32eb17182872d88d9bf8ee4d8c2dd5db6d95e2", + "executor": null, + "isExecuted": false, + "isSuccessful": null, + "ethGasPrice": null, + "gasUsed": null, + "fee": null, + "origin": null, + "dataDecoded": null, + "confirmationsRequired": null, + "confirmations": [ + { + "owner": "0xAd2e180019FCa9e55CADe76E4487F126Fd08DA34", + "submissionDate": "2020-09-15T21:54:49.663299Z", + "transactionHash": null, + "confirmationType": "CONFIRMATION", + "signature": "0x95a7250bb645f831c86defc847350e7faff815b2fb586282568e96cc859e39315876db20a2eed5f7a0412906ec5ab57652a6f645ad4833f345bda059b9da2b821c", + "signatureType": "EOA" + } + ], + "signatures": null + } + ], + "id": 67 +} +``` + +Not all fields are required, though. This method is really just a UX helper, which massages the +input to conform to the `EIP-712` [specification](https://docs.gnosis.io/safe/docs/contracts_tx_execution/#transaction-hash) +for the Gnosis Safe, and making the output be directly import:able to by a relay service. + ### 6.0.0 diff --git a/common/math/big.go b/common/math/big.go index 60f63f62f5de..1af5b4d879d6 100644 --- a/common/math/big.go +++ b/common/math/big.go @@ -90,10 +90,15 @@ func (i *Decimal256) UnmarshalText(input []byte) error { // MarshalText implements encoding.TextMarshaler. func (i *Decimal256) MarshalText() ([]byte, error) { + return []byte(i.String()), nil +} + +// String implements Stringer. +func (i *Decimal256) String() string { if i == nil { - return []byte("0"), nil + return "0" } - return []byte(fmt.Sprintf("%#d", (*big.Int)(i))), nil + return fmt.Sprintf("%#d", (*big.Int)(i)) } // ParseBig256 parses s as a 256 bit integer in decimal or hexadecimal syntax. diff --git a/signer/core/api.go b/signer/core/api.go index ff465aca5a61..3a1e7ee440cf 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -31,7 +31,6 @@ import ( "github.com/ethereum/go-ethereum/accounts/usbwallet" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" - "github.com/ethereum/go-ethereum/common/math" "github.com/ethereum/go-ethereum/internal/ethapi" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" @@ -42,7 +41,7 @@ const ( // numberOfAccountsToDerive For hardware wallets, the number of accounts to derive numberOfAccountsToDerive = 10 // ExternalAPIVersion -- see extapi_changelog.md - ExternalAPIVersion = "6.0.0" + ExternalAPIVersion = "6.1.0" // InternalAPIVersion -- see intapi_changelog.md InternalAPIVersion = "7.0.1" ) @@ -64,7 +63,7 @@ type ExternalAPI interface { // Version info about the APIs Version(ctx context.Context) (string, error) // SignGnosisSafeTransaction signs/confirms a gnosis-safe multisig transaction - SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) + SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) } // UIClientAPI specifies what method a UI needs to implement to be able to be used as a @@ -585,38 +584,9 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth } -// GnosisSigningResult conforms to the API required by the Gnosis Safe tx relay service. -// See 'SafeMultisigTransaction' on https://safe-transaction.mainnet.gnosis.io/ -type GnosisSigningResult struct { - Signature hexutil.Bytes `json:"signature"` - SafeTxHash common.Hash `json:"contractTransactionHash"` - Sender common.MixedcaseAddress `json:"sender"` - // Other fields that are required for the tx relay service - Safe common.MixedcaseAddress `json:"safe"` - To common.MixedcaseAddress `json:"to"` - Value math.Decimal256 `json:"value"` - GasPrice math.Decimal256 `json:"gasPrice"` - Data *hexutil.Bytes `json:"data"` - Operation uint8 `json:"operation"` - GasToken common.Address `json:"gasToken"` - RefundReceiver common.Address `json:"refundReceiver"` - BaseGas big.Int `json:"baseGas"` - SafeTxGas big.Int `json:"safeTxGas"` - Nonce big.Int `json:"nonce"` -} - -func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) { +func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) { // Do the usual validations, but on the last-stage transaction - args := &SendTxArgs{ - From: gnosisTx.Safe, - To: &gnosisTx.To, - Gas: hexutil.Uint64(gnosisTx.SafeTxGas.Uint64()), - GasPrice: hexutil.Big(gnosisTx.GasPrice), - Value: hexutil.Big(gnosisTx.Value), - Nonce: hexutil.Uint64(gnosisTx.Nonce.Uint64()), - Data: gnosisTx.Data, - Input: nil, - } + args := gnosisTx.ArgsForValidation() msgs, err := api.validator.ValidateTransaction(methodSelector, args) if err != nil { return nil, err @@ -627,98 +597,18 @@ func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.Mix return nil, err } } - - safeTxHash, preImage, err := accounts.GnosisSafeSigningHash(&gnosisTx) + typedData := gnosisTx.ToTypedData() + signature, preimage, err := api.signTypedData(ctx, signerAddress, typedData) if err != nil { return nil, err } - weival := big.Int(gnosisTx.Value) - var messages = []*NameValueType{ - &NameValueType{ - Name: "This is a signing-request for a Gnosis multisig transaction", - Value: "info", Typ: "", - }, - &NameValueType{ - Name: "Multisig address", - Value: gnosisTx.Safe.Address().String(), Typ: "address", - }, - &NameValueType{ - Name: " Value", - Value: fmt.Sprintf("%v wei", &weival), Typ: "uint256", - }, - &NameValueType{ - Name: " To", - Value: gnosisTx.To.String(), Typ: "address", - }, - &NameValueType{ - Name: " Gas price", - Value: gnosisTx.SafeTxGas.String(), Typ: "uint256", - }, - &NameValueType{ - Name: " Nonce", - Value: gnosisTx.Nonce.String(), Typ: "uint256", - }, - } - if gnosisTx.Data != nil { - messages = append(messages, &NameValueType{ - Name: " Data", - Value: gnosisTx.Data, Typ: "hexdata", - }) - } - - req := SignDataRequest{ - ContentType: "application/gnosis", - Address: signerAddress, - Rawdata: preImage, - Messages: messages, - Callinfo: msgs.Messages, - Hash: safeTxHash.Bytes(), - Meta: MetadataFromContext(ctx), - } - // Process approval - result, err := api.UI.ApproveSignData(&req) - if err != nil { - return nil, err - } - if !result.Approved { - return nil, ErrRequestDenied - } - account := accounts.Account{Address: signerAddress.Address()} - wallet, err := api.am.Find(account) - if err != nil { - return nil, err - } - pw, err := api.lookupOrQueryPassword(account.Address, - "Password for signing", - fmt.Sprintf("Please enter password for signing data with account %s", account.Address.Hex())) - if err != nil { - return nil, err - } - // Sign the data with the wallet - signature, err := wallet.SignDataWithPassphrase(account, pw, req.ContentType, req.Rawdata) - if err != nil { - return nil, err - } - if signature[64] < 2 { - signature[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper - } - checkSummedSender, _ := common.NewMixedcaseAddressFromString(account.Address.Hex()) - return &GnosisSigningResult{ - Signature: signature, - SafeTxHash: safeTxHash, - Sender: *checkSummedSender, // Must be checksummed - Safe: gnosisTx.Safe, - To: gnosisTx.To, - Value: gnosisTx.Value, // must be decimal string - GasPrice: gnosisTx.GasPrice, // must be decimal string - Data: gnosisTx.Data, - Operation: gnosisTx.Operation, - GasToken: gnosisTx.GasToken, - RefundReceiver: gnosisTx.RefundReceiver, - BaseGas: gnosisTx.BaseGas, - SafeTxGas: gnosisTx.SafeTxGas, - Nonce: gnosisTx.Nonce, - }, nil + checkSummedSender, _ := common.NewMixedcaseAddressFromString(signerAddress.Address().Hex()) + + gnosisTx.Signature = signature + gnosisTx.SafeTxHash = common.BytesToHash(preimage) + gnosisTx.Sender = *checkSummedSender // Must be checksumed to be accepted by relay + + return &gnosisTx, nil } // Returns the external api version. This method does not require user acceptance. Available methods are diff --git a/signer/core/auditlog.go b/signer/core/auditlog.go index a8cadf1a8d5c..926179612877 100644 --- a/signer/core/auditlog.go +++ b/signer/core/auditlog.go @@ -18,7 +18,6 @@ package core import ( "context" - "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -69,7 +68,7 @@ func (l *AuditLogger) SignData(ctx context.Context, contentType string, addr com return b, e } -func (l *AuditLogger) SignGnosisTx(ctx context.Context, addr common.MixedcaseAddress, gnosisTx accounts.GnosisSafeTx, methodSelector *string) (*GnosisSigningResult, error) { +func (l *AuditLogger) SignGnosisTx(ctx context.Context, addr common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) { sel := "" if methodSelector != nil { sel = *methodSelector diff --git a/signer/core/gnosis_safe.go b/signer/core/gnosis_safe.go new file mode 100644 index 000000000000..e4385f9dc37a --- /dev/null +++ b/signer/core/gnosis_safe.go @@ -0,0 +1,91 @@ +package core + +import ( + "fmt" + "math/big" + + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/common/math" +) + +// GnosisSafeTx is a type to parse the safe-tx returned by the relayer, +// it also conforms to the API required by the Gnosis Safe tx relay service. +// See 'SafeMultisigTransaction' on https://safe-transaction.mainnet.gnosis.io/ +type GnosisSafeTx struct { + // These fields are only used on output + Signature hexutil.Bytes `json:"signature"` + SafeTxHash common.Hash `json:"contractTransactionHash"` + Sender common.MixedcaseAddress `json:"sender"` + // These fields are used both on input and output + Safe common.MixedcaseAddress `json:"safe"` + To common.MixedcaseAddress `json:"to"` + Value math.Decimal256 `json:"value"` + GasPrice math.Decimal256 `json:"gasPrice"` + Data *hexutil.Bytes `json:"data"` + Operation uint8 `json:"operation"` + GasToken common.Address `json:"gasToken"` + RefundReceiver common.Address `json:"refundReceiver"` + BaseGas big.Int `json:"baseGas"` + SafeTxGas big.Int `json:"safeTxGas"` + Nonce big.Int `json:"nonce"` + InputExpHash common.Hash `json:"safeTxHash"` +} + +// ToTypedData converts the tx to a EIP-712 Typed Data structure for signing +func (tx *GnosisSafeTx) ToTypedData() TypedData { + var data hexutil.Bytes + if tx.Data != nil { + data = *tx.Data + } + gnosisTypedData := TypedData{ + Types: Types{ + "EIP712Domain": []Type{{Name: "verifyingContract", Type: "address"}}, + "SafeTx": []Type{ + {Name: "to", Type: "address"}, + {Name: "value", Type: "uint256"}, + {Name: "data", Type: "bytes"}, + {Name: "operation", Type: "uint8"}, + {Name: "safeTxGas", Type: "uint256"}, + {Name: "baseGas", Type: "uint256"}, + {Name: "gasPrice", Type: "uint256"}, + {Name: "gasToken", Type: "address"}, + {Name: "refundReceiver", Type: "address"}, + {Name: "nonce", Type: "uint256"}, + }, + }, + Domain: TypedDataDomain{ + VerifyingContract: tx.Safe.Address().Hex(), + }, + PrimaryType: "SafeTx", + Message: TypedDataMessage{ + "to": tx.To.Address().Hex(), + "value": tx.Value.String(), + "data": data, + "operation": fmt.Sprintf("%d", tx.Operation), + "safeTxGas": fmt.Sprintf("%#d", &tx.SafeTxGas), + "baseGas": fmt.Sprintf("%#d", &tx.BaseGas), + "gasPrice": tx.GasPrice.String(), + "gasToken": tx.GasToken.Hex(), + "refundReceiver": tx.RefundReceiver.Hex(), + "nonce": fmt.Sprintf("%d", tx.Nonce.Uint64()), + }, + } + return gnosisTypedData +} + +// ArgsForValidation returns a SendTxArgs struct, which can be used for the +// common validations, e.g. look up 4byte destinations +func (tx *GnosisSafeTx) ArgsForValidation() *SendTxArgs { + args := &SendTxArgs{ + From: tx.Safe, + To: &tx.To, + Gas: hexutil.Uint64(tx.SafeTxGas.Uint64()), + GasPrice: hexutil.Big(tx.GasPrice), + Value: hexutil.Big(tx.Value), + Nonce: hexutil.Uint64(tx.Nonce.Uint64()), + Data: tx.Data, + Input: nil, + } + return args +} diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 7fc66b4b740a..9168a892e011 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -125,7 +125,7 @@ var typedDataReferenceTypeRegexp = regexp.MustCompile(`^[A-Z](\w*)(\[\])?$`) // // Note, the produced signature conforms to the secp256k1 curve R, S and V values, // where the V value will be 27 or 28 for legacy reasons, if legacyV==true. -func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest, legacyV bool) (hexutil.Bytes, error) { +func (api *SignerAPI) sign(req *SignDataRequest, legacyV bool) (hexutil.Bytes, error) { // We make the request prior to looking up if we actually have the account, to prevent // account-enumeration via the API res, err := api.UI.ApproveSignData(req) @@ -136,7 +136,7 @@ func (api *SignerAPI) sign(addr common.MixedcaseAddress, req *SignDataRequest, l return nil, ErrRequestDenied } // Look up the wallet containing the requested signer - account := accounts.Account{Address: addr.Address()} + account := accounts.Account{Address: req.Address.Address()} wallet, err := api.am.Find(account) if err != nil { return nil, err @@ -167,7 +167,7 @@ func (api *SignerAPI) SignData(ctx context.Context, contentType string, addr com if err != nil { return nil, err } - signature, err := api.sign(addr, req, transformV) + signature, err := api.sign(req, transformV) if err != nil { api.UI.ShowError(err.Error()) return nil, err @@ -312,28 +312,38 @@ func cliqueHeaderHashAndRlp(header *types.Header) (hash, rlp []byte, err error) // SignTypedData signs EIP-712 conformant typed data // hash = keccak256("\x19${byteVersion}${domainSeparator}${hashStruct(message)}") +// It returns +// - the signature, +// - and/or any error func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, error) { + signature, _, err := api.signTypedData(ctx, addr, typedData) + return signature, err +} + +// signTypedData is identical to the capitalized version, except that it also returns the hash (preimage) +// - the signature preimage (hash) +func (api *SignerAPI) signTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, hexutil.Bytes, error) { domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) if err != nil { - return nil, err + return nil, nil, err } typedDataHash, err := typedData.HashStruct(typedData.PrimaryType, typedData.Message) if err != nil { - return nil, err + return nil, nil, err } rawData := []byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash))) sighash := crypto.Keccak256(rawData) messages, err := typedData.Format() if err != nil { - return nil, err + return nil, nil, err } - req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Messages: messages, Hash: sighash} - signature, err := api.sign(addr, req, true) + req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Messages: messages, Hash: sighash, Address: addr} + signature, err := api.sign(req, true) if err != nil { api.UI.ShowError(err.Error()) - return nil, err + return nil, nil, err } - return signature, nil + return signature, sighash, nil } // HashStruct generates a keccak256 hash of the encoding of the provided data @@ -420,8 +430,8 @@ func (typedData *TypedData) EncodeData(primaryType string, data map[string]inter buffer := bytes.Buffer{} // Verify extra data - if len(typedData.Types[primaryType]) < len(data) { - return nil, errors.New("there is extra data provided in the message") + if exp, got := len(typedData.Types[primaryType]), len(data); exp < got { + return nil, fmt.Errorf("there is extra data provided in the message (%d < %d)", exp, got) } // Add typehash diff --git a/signer/core/signed_data_test.go b/signer/core/signed_data_test.go index ab5f2cc9627d..23b7b9897bca 100644 --- a/signer/core/signed_data_test.go +++ b/signer/core/signed_data_test.go @@ -17,6 +17,7 @@ package core_test import ( + "bytes" "context" "encoding/json" "fmt" @@ -414,3 +415,119 @@ func TestFuzzerFiles(t *testing.T) { typedData.Format() } } + +var gnosisTypedData = ` +{ + "types": { + "EIP712Domain": [ + { "type": "address", "name": "verifyingContract" } + ], + "SafeTx": [ + { "type": "address", "name": "to" }, + { "type": "uint256", "name": "value" }, + { "type": "bytes", "name": "data" }, + { "type": "uint8", "name": "operation" }, + { "type": "uint256", "name": "safeTxGas" }, + { "type": "uint256", "name": "baseGas" }, + { "type": "uint256", "name": "gasPrice" }, + { "type": "address", "name": "gasToken" }, + { "type": "address", "name": "refundReceiver" }, + { "type": "uint256", "name": "nonce" } + ] + }, + "domain": { + "verifyingContract": "0x25a6c4BBd32B2424A9c99aEB0584Ad12045382B3" + }, + "primaryType": "SafeTx", + "message": { + "to": "0x9eE457023bB3De16D51A003a247BaEaD7fce313D", + "value": "20000000000000000", + "data": "0x", + "operation": 0, + "safeTxGas": 27845, + "baseGas": 0, + "gasPrice": "0", + "gasToken": "0x0000000000000000000000000000000000000000", + "refundReceiver": "0x0000000000000000000000000000000000000000", + "nonce": 3 + } +}` + +var gnosisTx = ` +{ + "safe": "0x25a6c4BBd32B2424A9c99aEB0584Ad12045382B3", + "to": "0x9eE457023bB3De16D51A003a247BaEaD7fce313D", + "value": "20000000000000000", + "data": null, + "operation": 0, + "gasToken": "0x0000000000000000000000000000000000000000", + "safeTxGas": 27845, + "baseGas": 0, + "gasPrice": "0", + "refundReceiver": "0x0000000000000000000000000000000000000000", + "nonce": 3, + "executionDate": null, + "submissionDate": "2020-09-15T21:59:23.815748Z", + "modified": "2020-09-15T21:59:23.815748Z", + "blockNumber": null, + "transactionHash": null, + "safeTxHash": "0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f", + "executor": null, + "isExecuted": false, + "isSuccessful": null, + "ethGasPrice": null, + "gasUsed": null, + "fee": null, + "origin": null, + "dataDecoded": null, + "confirmationsRequired": null, + "confirmations": [ + { + "owner": "0xAd2e180019FCa9e55CADe76E4487F126Fd08DA34", + "submissionDate": "2020-09-15T21:59:28.281243Z", + "transactionHash": null, + "confirmationType": "CONFIRMATION", + "signature": "0x5e562065a0cb15d766dac0cd49eb6d196a41183af302c4ecad45f1a81958d7797753f04424a9b0aa1cb0448e4ec8e189540fbcdda7530ef9b9d95dfc2d36cb521b", + "signatureType": "EOA" + } + ], + "signatures": null + } +` + +// TestGnosisTypedData tests the scenario where a user submits a full EIP-712 +// struct without using the gnosis-specific endpoint +func TestGnosisTypedData(t *testing.T) { + var td core.TypedData + err := json.Unmarshal([]byte(gnosisTypedData), &td) + if err != nil { + t.Fatalf("unmarshalling failed '%v'", err) + } + _, sighash, err := sign(td) + if err != nil { + t.Fatal(err) + } + expSigHash := common.FromHex("0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f") + if !bytes.Equal(expSigHash, sighash) { + t.Fatalf("Error, got %x, wanted %x", sighash, expSigHash) + } +} + +// TestGnosisCustomData tests the scenario where a user submits only the gnosis-safe +// specific data, and we fill the TypedData struct on our side +func TestGnosisCustomData(t *testing.T) { + var tx core.GnosisSafeTx + err := json.Unmarshal([]byte(gnosisTx), &tx) + if err != nil { + t.Fatal(err) + } + var td = tx.ToTypedData() + _, sighash, err := sign(td) + if err != nil { + t.Fatal(err) + } + expSigHash := common.FromHex("0x28bae2bd58d894a1d9b69e5e9fde3570c4b98a6fc5499aefb54fb830137e831f") + if !bytes.Equal(expSigHash, sighash) { + t.Fatalf("Error, got %x, wanted %x", sighash, expSigHash) + } +} From 346b8fafe827f937ced9e0f4af55067c2b35f2b7 Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Thu, 24 Sep 2020 12:30:37 +0200 Subject: [PATCH 5/7] signer: fix auditlog print, change rpc-name (signGnosisTx to signGnosisSafeTx) --- cmd/clef/extapi_changelog.md | 6 +++--- signer/core/api.go | 4 ++-- signer/core/auditlog.go | 18 +++++++++++------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/clef/extapi_changelog.md b/cmd/clef/extapi_changelog.md index 6bb068430716..31554f079020 100644 --- a/cmd/clef/extapi_changelog.md +++ b/cmd/clef/extapi_changelog.md @@ -12,13 +12,13 @@ Additional labels for pre-release and build metadata are available as extensions ### 6.1.0 -The API-method `account_signGnosisTx` was added. This method takes two parameters, +The API-method `account_signGnosisSafeTx` was added. This method takes two parameters, `[address, safeTx]`. The latter, `safeTx`, can be copy-pasted from the gnosis relay. For example: ``` { "jsonrpc": "2.0", - "method": "account_signGnosisTx", + "method": "account_signGnosisSafeTx", "params": ["0xfd1c4226bfD1c436672092F4eCbfC270145b7256", { "safe": "0x25a6c4BBd32B2424A9c99aEB0584Ad12045382B3", @@ -66,7 +66,7 @@ The API-method `account_signGnosisTx` was added. This method takes two parameter Not all fields are required, though. This method is really just a UX helper, which massages the input to conform to the `EIP-712` [specification](https://docs.gnosis.io/safe/docs/contracts_tx_execution/#transaction-hash) -for the Gnosis Safe, and making the output be directly import:able to by a relay service. +for the Gnosis Safe, and making the output be directly importable to by a relay service. ### 6.0.0 diff --git a/signer/core/api.go b/signer/core/api.go index 3a1e7ee440cf..2657d09b0816 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -63,7 +63,7 @@ type ExternalAPI interface { // Version info about the APIs Version(ctx context.Context) (string, error) // SignGnosisSafeTransaction signs/confirms a gnosis-safe multisig transaction - SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) + SignGnosisSafeTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) } // UIClientAPI specifies what method a UI needs to implement to be able to be used as a @@ -584,7 +584,7 @@ func (api *SignerAPI) SignTransaction(ctx context.Context, args SendTxArgs, meth } -func (api *SignerAPI) SignGnosisTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) { +func (api *SignerAPI) SignGnosisSafeTx(ctx context.Context, signerAddress common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) { // Do the usual validations, but on the last-stage transaction args := gnosisTx.ArgsForValidation() msgs, err := api.validator.ValidateTransaction(methodSelector, args) diff --git a/signer/core/auditlog.go b/signer/core/auditlog.go index 926179612877..bda88a8b2e55 100644 --- a/signer/core/auditlog.go +++ b/signer/core/auditlog.go @@ -18,6 +18,7 @@ package core import ( "context" + "encoding/json" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" @@ -61,25 +62,28 @@ func (l *AuditLogger) SignTransaction(ctx context.Context, args SendTxArgs, meth } func (l *AuditLogger) SignData(ctx context.Context, contentType string, addr common.MixedcaseAddress, data interface{}) (hexutil.Bytes, error) { + marshalledData, _ := json.Marshal(data) // can ignore error, marshalling what we just unmarshalled l.log.Info("SignData", "type", "request", "metadata", MetadataFromContext(ctx).String(), - "addr", addr.String(), "data", data, "content-type", contentType) + "addr", addr.String(), "data", marshalledData, "content-type", contentType) b, e := l.api.SignData(ctx, contentType, addr, data) l.log.Info("SignData", "type", "response", "data", common.Bytes2Hex(b), "error", e) return b, e } -func (l *AuditLogger) SignGnosisTx(ctx context.Context, addr common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) { +func (l *AuditLogger) SignGnosisSafeTx(ctx context.Context, addr common.MixedcaseAddress, gnosisTx GnosisSafeTx, methodSelector *string) (*GnosisSafeTx, error) { sel := "" if methodSelector != nil { sel = *methodSelector } - l.log.Info("SignGnosisTx", "type", "request", "metadata", MetadataFromContext(ctx).String(), - "addr", addr.String(), "data", gnosisTx, "selector", sel) - res, e := l.api.SignGnosisTx(ctx, addr, gnosisTx, methodSelector) + data, _ := json.Marshal(gnosisTx) // can ignore error, marshalling what we just unmarshalled + l.log.Info("SignGnosisSafeTx", "type", "request", "metadata", MetadataFromContext(ctx).String(), + "addr", addr.String(), "data", string(data), "selector", sel) + res, e := l.api.SignGnosisSafeTx(ctx, addr, gnosisTx, methodSelector) if res != nil { - l.log.Info("SignGnosisTx", "type", "response", "data", common.Bytes2Hex(res.Signature), "error", e) + data, _ := json.Marshal(res) // can ignore error, marshalling what we just unmarshalled + l.log.Info("SignGnosisSafeTx", "type", "response", "data", string(data), "error", e) } else { - l.log.Info("SignTransaction", "type", "response", "data", res, "error", e) + l.log.Info("SignGnosisSafeTx", "type", "response", "data", res, "error", e) } return res, e } From f09e3de540a70fa60f8c6c9d0fa3ba3ed61ea41e Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 28 Sep 2020 15:19:06 +0200 Subject: [PATCH 6/7] signer: pass validation-messages/warnings to the UI for gnonsis-safe txs --- signer/core/api.go | 2 +- signer/core/signed_data.go | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/signer/core/api.go b/signer/core/api.go index 2657d09b0816..6b7d18dfde2c 100644 --- a/signer/core/api.go +++ b/signer/core/api.go @@ -598,7 +598,7 @@ func (api *SignerAPI) SignGnosisSafeTx(ctx context.Context, signerAddress common } } typedData := gnosisTx.ToTypedData() - signature, preimage, err := api.signTypedData(ctx, signerAddress, typedData) + signature, preimage, err := api.signTypedData(ctx, signerAddress, typedData, msgs) if err != nil { return nil, err } diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 9168a892e011..65eab679608d 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -316,13 +316,14 @@ func cliqueHeaderHashAndRlp(header *types.Header) (hash, rlp []byte, err error) // - the signature, // - and/or any error func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, error) { - signature, _, err := api.signTypedData(ctx, addr, typedData) + signature, _, err := api.signTypedData(ctx, addr, typedData, nil) return signature, err } // signTypedData is identical to the capitalized version, except that it also returns the hash (preimage) // - the signature preimage (hash) -func (api *SignerAPI) signTypedData(ctx context.Context, addr common.MixedcaseAddress, typedData TypedData) (hexutil.Bytes, hexutil.Bytes, error) { +func (api *SignerAPI) signTypedData(ctx context.Context, addr common.MixedcaseAddress, + typedData TypedData, validationMessages *ValidationMessages) (hexutil.Bytes, hexutil.Bytes, error) { domainSeparator, err := typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) if err != nil { return nil, nil, err @@ -337,6 +338,15 @@ func (api *SignerAPI) signTypedData(ctx context.Context, addr common.MixedcaseAd if err != nil { return nil, nil, err } + if validationMessages != nil { + for _, validationMsg := range validationMessages.Messages { + messages = append(messages, &NameValueType{ + Name: "Validation message", + Value: validationMsg.Message, + Typ: validationMsg.Typ, + }) + } + } req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Messages: messages, Hash: sighash, Address: addr} signature, err := api.sign(req, true) if err != nil { @@ -844,7 +854,11 @@ func (nvt *NameValueType) Pprint(depth int) string { output.WriteString(sublevel) } } else { - output.WriteString(fmt.Sprintf("%q\n", nvt.Value)) + if nvt.Value != nil { + output.WriteString(fmt.Sprintf("%q\n", nvt.Value)) + } else { + output.WriteString("\n") + } } return output.String() } From 8f8af8b4fdf5bd64d8e3f22f9262278828a04cdc Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 29 Sep 2020 16:21:48 +0200 Subject: [PATCH 7/7] signer/core: minor change to validationmessages of typed data --- signer/core/cliui.go | 7 +++++++ signer/core/signed_data.go | 15 +++++++-------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/signer/core/cliui.go b/signer/core/cliui.go index 27a2f71aaaf7..cbfb56c9df9e 100644 --- a/signer/core/cliui.go +++ b/signer/core/cliui.go @@ -148,6 +148,13 @@ func (ui *CommandlineUI) ApproveSignData(request *SignDataRequest) (SignDataResp fmt.Printf("-------- Sign data request--------------\n") fmt.Printf("Account: %s\n", request.Address.String()) + if len(request.Callinfo) != 0 { + fmt.Printf("\nValidation messages:\n") + for _, m := range request.Callinfo { + fmt.Printf(" * %s : %s\n", m.Typ, m.Message) + } + fmt.Println() + } fmt.Printf("messages:\n") for _, nvt := range request.Messages { fmt.Printf("\u00a0\u00a0%v\n", strings.TrimSpace(nvt.Pprint(1))) diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 65eab679608d..19377a521b13 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -338,16 +338,15 @@ func (api *SignerAPI) signTypedData(ctx context.Context, addr common.MixedcaseAd if err != nil { return nil, nil, err } + req := &SignDataRequest{ + ContentType: DataTyped.Mime, + Rawdata: rawData, + Messages: messages, + Hash: sighash, + Address: addr} if validationMessages != nil { - for _, validationMsg := range validationMessages.Messages { - messages = append(messages, &NameValueType{ - Name: "Validation message", - Value: validationMsg.Message, - Typ: validationMsg.Typ, - }) - } + req.Callinfo = validationMessages.Messages } - req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Messages: messages, Hash: sighash, Address: addr} signature, err := api.sign(req, true) if err != nil { api.UI.ShowError(err.Error())