diff --git a/README.md b/README.md index 64f272f1a6..1e8dba8090 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ Golang execution layer implementation of the Ethereum protocol. https://pkg.go.dev/badge/github.com/ethereum/go-ethereum )](https://pkg.go.dev/github.com/ethereum/go-ethereum?tab=doc) [![Go Report Card](https://goreportcard.com/badge/github.com/ethereum/go-ethereum)](https://goreportcard.com/report/github.com/ethereum/go-ethereum) -[![Travis](https://travis-ci.com/ethereum/go-ethereum.svg?branch=master)](https://travis-ci.com/ethereum/go-ethereum) +[![Travis](https://app.travis-ci.com/ethereum/go-ethereum.svg?branch=master)](https://app.travis-ci.com/github/ethereum/go-ethereum) [![Discord](https://img.shields.io/badge/discord-join%20chat-blue.svg)](https://discord.gg/nthXNEv) Automated builds are available for stable releases and the unstable master branch. Binary diff --git a/beacon/engine/types.go b/beacon/engine/types.go index 67f30d4455..f72319ad50 100644 --- a/beacon/engine/types.go +++ b/beacon/engine/types.go @@ -26,6 +26,16 @@ import ( "github.com/ethereum/go-ethereum/trie" ) +// PayloadVersion denotes the version of PayloadAttributes used to request the +// building of the payload to commence. +type PayloadVersion byte + +var ( + PayloadV1 PayloadVersion = 0x1 + PayloadV2 PayloadVersion = 0x2 + PayloadV3 PayloadVersion = 0x3 +) + //go:generate go run github.com/fjl/gencodec -type PayloadAttributes -field-override payloadAttributesMarshaling -out gen_blockparams.go // PayloadAttributes describes the environment context in which a block should @@ -115,6 +125,21 @@ type TransitionConfigurationV1 struct { // PayloadID is an identifier of the payload build process type PayloadID [8]byte +// Version returns the payload version associated with the identifier. +func (b PayloadID) Version() PayloadVersion { + return PayloadVersion(b[0]) +} + +// Is returns whether the identifier matches any of provided payload versions. +func (b PayloadID) Is(versions ...PayloadVersion) bool { + for _, v := range versions { + if v == b.Version() { + return true + } + } + return false +} + func (b PayloadID) String() string { return hexutil.Encode(b[:]) } diff --git a/core/types/transaction.go b/core/types/transaction.go index 9ec0199a03..7d2e9d5325 100644 --- a/core/types/transaction.go +++ b/core/types/transaction.go @@ -19,6 +19,7 @@ package types import ( "bytes" "errors" + "fmt" "io" "math/big" "sync/atomic" @@ -320,6 +321,7 @@ func (tx *Transaction) Cost() *big.Int { // RawSignatureValues returns the V, R, S signature values of the transaction. // The return values should not be modified by the caller. +// The return values may be nil or zero, if the transaction is unsigned. func (tx *Transaction) RawSignatureValues() (v, r, s *big.Int) { return tx.inner.rawSignatureValues() } @@ -508,6 +510,9 @@ func (tx *Transaction) WithSignature(signer Signer, sig []byte) (*Transaction, e if err != nil { return nil, err } + if r == nil || s == nil || v == nil { + return nil, fmt.Errorf("%w: r: %s, s: %s, v: %s", ErrInvalidSig, r, s, v) + } cpy := tx.inner.copy() cpy.setSignatureValues(signer.ChainID(), v, r, s) return &Transaction{inner: cpy, time: tx.time}, nil diff --git a/core/types/transaction_signing_test.go b/core/types/transaction_signing_test.go index 2a9ceb0952..61b78fe029 100644 --- a/core/types/transaction_signing_test.go +++ b/core/types/transaction_signing_test.go @@ -18,11 +18,13 @@ package types import ( "errors" + "fmt" "math/big" "testing" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rlp" ) @@ -136,3 +138,53 @@ func TestChainId(t *testing.T) { t.Error("expected no error") } } + +type nilSigner struct { + v, r, s *big.Int + Signer +} + +func (ns *nilSigner) SignatureValues(tx *Transaction, sig []byte) (r, s, v *big.Int, err error) { + return ns.v, ns.r, ns.s, nil +} + +// TestNilSigner ensures a faulty Signer implementation does not result in nil signature values or panics. +func TestNilSigner(t *testing.T) { + key, _ := crypto.GenerateKey() + innerSigner := LatestSignerForChainID(big.NewInt(1)) + for i, signer := range []Signer{ + &nilSigner{v: nil, r: nil, s: nil, Signer: innerSigner}, + &nilSigner{v: big.NewInt(1), r: big.NewInt(1), s: nil, Signer: innerSigner}, + &nilSigner{v: big.NewInt(1), r: nil, s: big.NewInt(1), Signer: innerSigner}, + &nilSigner{v: nil, r: big.NewInt(1), s: big.NewInt(1), Signer: innerSigner}, + } { + t.Run(fmt.Sprintf("signer_%d", i), func(t *testing.T) { + t.Run("legacy", func(t *testing.T) { + legacyTx := createTestLegacyTxInner() + _, err := SignNewTx(key, signer, legacyTx) + if !errors.Is(err, ErrInvalidSig) { + t.Fatal("expected signature values error, no nil result or panic") + } + }) + // test Blob tx specifically, since the signature value types changed + t.Run("blobtx", func(t *testing.T) { + blobtx := createEmptyBlobTxInner(false) + _, err := SignNewTx(key, signer, blobtx) + if !errors.Is(err, ErrInvalidSig) { + t.Fatal("expected signature values error, no nil result or panic") + } + }) + }) + } +} + +func createTestLegacyTxInner() *LegacyTx { + return &LegacyTx{ + Nonce: uint64(0), + To: nil, + Value: big.NewInt(0), + Gas: params.TxGas, + GasPrice: big.NewInt(params.GWei), + Data: nil, + } +} diff --git a/core/types/tx_blob_test.go b/core/types/tx_blob_test.go index 44ac48cc6f..25d09e31ce 100644 --- a/core/types/tx_blob_test.go +++ b/core/types/tx_blob_test.go @@ -65,6 +65,12 @@ var ( ) func createEmptyBlobTx(key *ecdsa.PrivateKey, withSidecar bool) *Transaction { + blobtx := createEmptyBlobTxInner(withSidecar) + signer := NewCancunSigner(blobtx.ChainID.ToBig()) + return MustSignNewTx(key, signer, blobtx) +} + +func createEmptyBlobTxInner(withSidecar bool) *BlobTx { sidecar := &BlobTxSidecar{ Blobs: []kzg4844.Blob{emptyBlob}, Commitments: []kzg4844.Commitment{emptyBlobCommit}, @@ -85,6 +91,5 @@ func createEmptyBlobTx(key *ecdsa.PrivateKey, withSidecar bool) *Transaction { if withSidecar { blobtx.Sidecar = sidecar } - signer := NewCancunSigner(blobtx.ChainID.ToBig()) - return MustSignNewTx(key, signer, blobtx) + return blobtx } diff --git a/docs/postmortems/2021-08-22-split-postmortem.md b/docs/postmortems/2021-08-22-split-postmortem.md index 962aa51f64..0986f00b65 100644 --- a/docs/postmortems/2021-08-22-split-postmortem.md +++ b/docs/postmortems/2021-08-22-split-postmortem.md @@ -87,7 +87,7 @@ The blocks on the 'bad' chain were investigated, and Tim Beiko reached out to th ### Disclosure decision -The geth-team have an official policy regarding [vulnerability disclosure](https://geth.ethereum.org/docs/vulnerabilities/vulnerabilities). +The geth-team have an official policy regarding [vulnerability disclosure](https://geth.ethereum.org/docs/developers/geth-developer/disclosures). > The primary goal for the Geth team is the health of the Ethereum network as a whole, and the decision whether or not to publish details about a serious vulnerability boils down to minimizing the risk and/or impact of discovery and exploitation. diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index f02b5f3622..c48a7d0e49 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -173,33 +173,41 @@ func newConsensusAPIWithoutHeartbeat(eth *eth.Ethereum) *ConsensusAPI { // and return its payloadID. func (api *ConsensusAPI) ForkchoiceUpdatedV1(update engine.ForkchoiceStateV1, payloadAttributes *engine.PayloadAttributes) (engine.ForkChoiceResponse, error) { if payloadAttributes != nil { - if payloadAttributes.Withdrawals != nil { - return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("withdrawals not supported in V1")) + if payloadAttributes.Withdrawals != nil || payloadAttributes.BeaconRoot != nil { + return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("withdrawals and beacon root not supported in V1")) } if api.eth.BlockChain().Config().IsShanghai(api.eth.BlockChain().Config().LondonBlock, payloadAttributes.Timestamp) { return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("forkChoiceUpdateV1 called post-shanghai")) } } - return api.forkchoiceUpdated(update, payloadAttributes, false) + return api.forkchoiceUpdated(update, payloadAttributes, engine.PayloadV1, false) } -// ForkchoiceUpdatedV2 is equivalent to V1 with the addition of withdrawals in the payload attributes. +// ForkchoiceUpdatedV2 is equivalent to V1 with the addition of withdrawals in the payload +// attributes. It supports both PayloadAttributesV1 and PayloadAttributesV2. func (api *ConsensusAPI) ForkchoiceUpdatedV2(update engine.ForkchoiceStateV1, params *engine.PayloadAttributes) (engine.ForkChoiceResponse, error) { if params != nil { - if params.Withdrawals == nil { - return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("missing withdrawals")) + switch api.eth.BlockChain().Config().LatestFork(params.Timestamp) { + case forks.Paris: + if params.Withdrawals != nil { + return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("withdrawals before shanghai")) + } + case forks.Shanghai: + if params.Withdrawals == nil { + return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("missing withdrawals")) + } + default: + return engine.STATUS_INVALID, engine.UnsupportedFork.With(errors.New("forkchoiceUpdatedV2 must only be called with paris and shanghai payloads")) } if params.BeaconRoot != nil { return engine.STATUS_INVALID, engine.InvalidParams.With(errors.New("unexpected beacon root")) } - if api.eth.BlockChain().Config().LatestFork(params.Timestamp) != forks.Shanghai { - return engine.STATUS_INVALID, engine.UnsupportedFork.With(errors.New("forkchoiceUpdatedV2 must only be called for shanghai payloads")) - } } - return api.forkchoiceUpdated(update, params, false) + return api.forkchoiceUpdated(update, params, engine.PayloadV2, false) } -// ForkchoiceUpdatedV3 is equivalent to V2 with the addition of parent beacon block root in the payload attributes. +// ForkchoiceUpdatedV3 is equivalent to V2 with the addition of parent beacon block root +// in the payload attributes. It supports only PayloadAttributesV3. func (api *ConsensusAPI) ForkchoiceUpdatedV3(update engine.ForkchoiceStateV1, params *engine.PayloadAttributes) (engine.ForkChoiceResponse, error) { if params != nil { // TODO(matt): according to https://github.com/ethereum/execution-apis/pull/498, @@ -220,10 +228,10 @@ func (api *ConsensusAPI) ForkchoiceUpdatedV3(update engine.ForkchoiceStateV1, pa // hash, even if params are wrong. To do this we need to split up // forkchoiceUpdate into a function that only updates the head and then a // function that kicks off block construction. - return api.forkchoiceUpdated(update, params, false) + return api.forkchoiceUpdated(update, params, engine.PayloadV3, false) } -func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payloadAttributes *engine.PayloadAttributes, simulatorMode bool) (engine.ForkChoiceResponse, error) { +func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payloadAttributes *engine.PayloadAttributes, payloadVersion engine.PayloadVersion, simulatorMode bool) (engine.ForkChoiceResponse, error) { api.forkchoiceLock.Lock() defer api.forkchoiceLock.Unlock() @@ -367,6 +375,7 @@ func (api *ConsensusAPI) forkchoiceUpdated(update engine.ForkchoiceStateV1, payl Random: payloadAttributes.Random, Withdrawals: payloadAttributes.Withdrawals, BeaconRoot: payloadAttributes.BeaconRoot, + Version: payloadVersion, } id := args.Id() // If we already are busy generating this work, then we do not need @@ -430,6 +439,9 @@ func (api *ConsensusAPI) ExchangeTransitionConfigurationV1(config engine.Transit // GetPayloadV1 returns a cached payload by id. func (api *ConsensusAPI) GetPayloadV1(payloadID engine.PayloadID) (*engine.ExecutableData, error) { + if !payloadID.Is(engine.PayloadV1) { + return nil, engine.UnsupportedFork + } data, err := api.getPayload(payloadID, false) if err != nil { return nil, err @@ -439,11 +451,17 @@ func (api *ConsensusAPI) GetPayloadV1(payloadID engine.PayloadID) (*engine.Execu // GetPayloadV2 returns a cached payload by id. func (api *ConsensusAPI) GetPayloadV2(payloadID engine.PayloadID) (*engine.ExecutionPayloadEnvelope, error) { + if !payloadID.Is(engine.PayloadV1, engine.PayloadV2) { + return nil, engine.UnsupportedFork + } return api.getPayload(payloadID, false) } // GetPayloadV3 returns a cached payload by id. func (api *ConsensusAPI) GetPayloadV3(payloadID engine.PayloadID) (*engine.ExecutionPayloadEnvelope, error) { + if !payloadID.Is(engine.PayloadV3) { + return nil, engine.UnsupportedFork + } return api.getPayload(payloadID, false) } diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index 07b6c3f7a9..f1d48d0dea 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -210,6 +210,7 @@ func TestEth2PrepareAndGetPayload(t *testing.T) { FeeRecipient: blockParams.SuggestedFeeRecipient, Random: blockParams.Random, BeaconRoot: blockParams.BeaconRoot, + Version: engine.PayloadV1, }).Id() execData, err := api.GetPayloadV1(payloadID) if err != nil { @@ -1076,6 +1077,7 @@ func TestWithdrawals(t *testing.T) { Random: blockParams.Random, Withdrawals: blockParams.Withdrawals, BeaconRoot: blockParams.BeaconRoot, + Version: engine.PayloadV2, }).Id() execData, err := api.GetPayloadV2(payloadID) if err != nil { @@ -1124,6 +1126,7 @@ func TestWithdrawals(t *testing.T) { Random: blockParams.Random, Withdrawals: blockParams.Withdrawals, BeaconRoot: blockParams.BeaconRoot, + Version: engine.PayloadV2, }).Id() execData, err = api.GetPayloadV2(payloadID) if err != nil { @@ -1238,12 +1241,15 @@ func TestNilWithdrawals(t *testing.T) { for _, test := range tests { var ( - err error - shanghai = genesis.Config.IsShanghai(genesis.Config.LondonBlock, test.blockParams.Timestamp) + err error + payloadVersion engine.PayloadVersion + shanghai = genesis.Config.IsShanghai(genesis.Config.LondonBlock, test.blockParams.Timestamp) ) if !shanghai { + payloadVersion = engine.PayloadV1 _, err = api.ForkchoiceUpdatedV1(fcState, &test.blockParams) } else { + payloadVersion = engine.PayloadV2 _, err = api.ForkchoiceUpdatedV2(fcState, &test.blockParams) } if test.wantErr { @@ -1262,6 +1268,7 @@ func TestNilWithdrawals(t *testing.T) { Timestamp: test.blockParams.Timestamp, FeeRecipient: test.blockParams.SuggestedFeeRecipient, Random: test.blockParams.Random, + Version: payloadVersion, }).Id() execData, err := api.GetPayloadV2(payloadID) if err != nil { @@ -1616,6 +1623,7 @@ func TestParentBeaconBlockRoot(t *testing.T) { Random: blockParams.Random, Withdrawals: blockParams.Withdrawals, BeaconRoot: blockParams.BeaconRoot, + Version: engine.PayloadV3, }).Id() execData, err := api.GetPayloadV3(payloadID) if err != nil { diff --git a/eth/catalyst/simulated_beacon.go b/eth/catalyst/simulated_beacon.go index f55fe0813a..5ad50f14c1 100644 --- a/eth/catalyst/simulated_beacon.go +++ b/eth/catalyst/simulated_beacon.go @@ -160,7 +160,7 @@ func (c *SimulatedBeacon) sealBlock(withdrawals []*types.Withdrawal, timestamp u SuggestedFeeRecipient: feeRecipient, Withdrawals: withdrawals, Random: random, - }, true) + }, engine.PayloadV2, true) if err != nil { return err } diff --git a/ethclient/ethclient.go b/ethclient/ethclient.go index 5b4e906cbb..4c63b776ef 100644 --- a/ethclient/ethclient.go +++ b/ethclient/ethclient.go @@ -677,18 +677,20 @@ type rpcProgress struct { PulledStates hexutil.Uint64 KnownStates hexutil.Uint64 - SyncedAccounts hexutil.Uint64 - SyncedAccountBytes hexutil.Uint64 - SyncedBytecodes hexutil.Uint64 - SyncedBytecodeBytes hexutil.Uint64 - SyncedStorage hexutil.Uint64 - SyncedStorageBytes hexutil.Uint64 - HealedTrienodes hexutil.Uint64 - HealedTrienodeBytes hexutil.Uint64 - HealedBytecodes hexutil.Uint64 - HealedBytecodeBytes hexutil.Uint64 - HealingTrienodes hexutil.Uint64 - HealingBytecode hexutil.Uint64 + SyncedAccounts hexutil.Uint64 + SyncedAccountBytes hexutil.Uint64 + SyncedBytecodes hexutil.Uint64 + SyncedBytecodeBytes hexutil.Uint64 + SyncedStorage hexutil.Uint64 + SyncedStorageBytes hexutil.Uint64 + HealedTrienodes hexutil.Uint64 + HealedTrienodeBytes hexutil.Uint64 + HealedBytecodes hexutil.Uint64 + HealedBytecodeBytes hexutil.Uint64 + HealingTrienodes hexutil.Uint64 + HealingBytecode hexutil.Uint64 + TxIndexFinishedBlocks hexutil.Uint64 + TxIndexRemainingBlocks hexutil.Uint64 } func (p *rpcProgress) toSyncProgress() *ethereum.SyncProgress { @@ -696,22 +698,24 @@ func (p *rpcProgress) toSyncProgress() *ethereum.SyncProgress { return nil } return ðereum.SyncProgress{ - StartingBlock: uint64(p.StartingBlock), - CurrentBlock: uint64(p.CurrentBlock), - HighestBlock: uint64(p.HighestBlock), - PulledStates: uint64(p.PulledStates), - KnownStates: uint64(p.KnownStates), - SyncedAccounts: uint64(p.SyncedAccounts), - SyncedAccountBytes: uint64(p.SyncedAccountBytes), - SyncedBytecodes: uint64(p.SyncedBytecodes), - SyncedBytecodeBytes: uint64(p.SyncedBytecodeBytes), - SyncedStorage: uint64(p.SyncedStorage), - SyncedStorageBytes: uint64(p.SyncedStorageBytes), - HealedTrienodes: uint64(p.HealedTrienodes), - HealedTrienodeBytes: uint64(p.HealedTrienodeBytes), - HealedBytecodes: uint64(p.HealedBytecodes), - HealedBytecodeBytes: uint64(p.HealedBytecodeBytes), - HealingTrienodes: uint64(p.HealingTrienodes), - HealingBytecode: uint64(p.HealingBytecode), + StartingBlock: uint64(p.StartingBlock), + CurrentBlock: uint64(p.CurrentBlock), + HighestBlock: uint64(p.HighestBlock), + PulledStates: uint64(p.PulledStates), + KnownStates: uint64(p.KnownStates), + SyncedAccounts: uint64(p.SyncedAccounts), + SyncedAccountBytes: uint64(p.SyncedAccountBytes), + SyncedBytecodes: uint64(p.SyncedBytecodes), + SyncedBytecodeBytes: uint64(p.SyncedBytecodeBytes), + SyncedStorage: uint64(p.SyncedStorage), + SyncedStorageBytes: uint64(p.SyncedStorageBytes), + HealedTrienodes: uint64(p.HealedTrienodes), + HealedTrienodeBytes: uint64(p.HealedTrienodeBytes), + HealedBytecodes: uint64(p.HealedBytecodes), + HealedBytecodeBytes: uint64(p.HealedBytecodeBytes), + HealingTrienodes: uint64(p.HealingTrienodes), + HealingBytecode: uint64(p.HealingBytecode), + TxIndexFinishedBlocks: uint64(p.TxIndexFinishedBlocks), + TxIndexRemainingBlocks: uint64(p.TxIndexRemainingBlocks), } } diff --git a/ethclient/ethclient_test.go b/ethclient/ethclient_test.go index 2ef68337c6..fd053c1d73 100644 --- a/ethclient/ethclient_test.go +++ b/ethclient/ethclient_test.go @@ -231,6 +231,13 @@ func newTestBackend(t *testing.T) (*node.Node, []*types.Block) { if _, err := ethservice.BlockChain().InsertChain(blocks[1:]); err != nil { t.Fatalf("can't import test blocks: %v", err) } + // Ensure the tx indexing is fully generated + for ; ; time.Sleep(time.Millisecond * 100) { + progress, err := ethservice.BlockChain().TxIndexProgress() + if err == nil && progress.Done() { + break + } + } return n, blocks } diff --git a/internal/flags/helpers.go b/internal/flags/helpers.go index 369a931e8a..0112724fa1 100644 --- a/internal/flags/helpers.go +++ b/internal/flags/helpers.go @@ -115,7 +115,7 @@ func doMigrateFlags(ctx *cli.Context) { for _, parent := range ctx.Lineage()[1:] { if parent.IsSet(name) { // When iterating across the lineage, we will be served both - // the 'canon' and alias formats of all commmands. In most cases, + // the 'canon' and alias formats of all commands. In most cases, // it's fine to set it in the ctx multiple times (one for each // name), however, the Slice-flags are not fine. // The slice-flags accumulate, so if we set it once as diff --git a/miner/payload_building.go b/miner/payload_building.go index 69ffab75b5..719736c479 100644 --- a/miner/payload_building.go +++ b/miner/payload_building.go @@ -35,12 +35,13 @@ import ( // Check engine-api specification for more details. // https://github.com/ethereum/execution-apis/blob/main/src/engine/cancun.md#payloadattributesv3 type BuildPayloadArgs struct { - Parent common.Hash // The parent block to build payload on top - Timestamp uint64 // The provided timestamp of generated payload - FeeRecipient common.Address // The provided recipient address for collecting transaction fee - Random common.Hash // The provided randomness value - Withdrawals types.Withdrawals // The provided withdrawals - BeaconRoot *common.Hash // The provided beaconRoot (Cancun) + Parent common.Hash // The parent block to build payload on top + Timestamp uint64 // The provided timestamp of generated payload + FeeRecipient common.Address // The provided recipient address for collecting transaction fee + Random common.Hash // The provided randomness value + Withdrawals types.Withdrawals // The provided withdrawals + BeaconRoot *common.Hash // The provided beaconRoot (Cancun) + Version engine.PayloadVersion // Versioning byte for payload id calculation. } // Id computes an 8-byte identifier by hashing the components of the payload arguments. @@ -57,6 +58,7 @@ func (args *BuildPayloadArgs) Id() engine.PayloadID { } var out engine.PayloadID copy(out[:], hasher.Sum(nil)[:8]) + out[0] = byte(args.Version) return out } diff --git a/params/version.go b/params/version.go index ba8a0f50d5..a18d6dc914 100644 --- a/params/version.go +++ b/params/version.go @@ -23,7 +23,7 @@ import ( const ( VersionMajor = 1 // Major version component of the current release VersionMinor = 13 // Minor version component of the current release - VersionPatch = 11 // Patch version component of the current release + VersionPatch = 12 // Patch version component of the current release VersionMeta = "unstable" // Version metadata to append to the version string )