From 853e0c23f36579423dbac8b4bcb9eeedb53daa9b Mon Sep 17 00:00:00 2001 From: Martin HS Date: Tue, 23 Apr 2024 10:33:36 +0200 Subject: [PATCH 01/15] eth/catalyst, trie/pathdb: fix flaky tests (#29571) This change fixes three flaky tests `TestEth2AssembleBlock`,`TestEth2NewBlock`, `TestEth2PrepareAndGetPayload` and `TestDisable`. --------- Co-authored-by: Gary Rong --- eth/catalyst/api_test.go | 8 ++++---- triedb/pathdb/database.go | 1 + triedb/pathdb/database_test.go | 6 +++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index ab1d78f90e..b8645d6be4 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -112,7 +112,7 @@ func TestEth2AssembleBlock(t *testing.T) { if err != nil { t.Fatalf("error signing transaction, err=%v", err) } - ethservice.TxPool().Add([]*types.Transaction{tx}, true, false) + ethservice.TxPool().Add([]*types.Transaction{tx}, true, true) blockParams := engine.PayloadAttributes{ Timestamp: blocks[9].Time() + 5, } @@ -189,7 +189,7 @@ func TestEth2PrepareAndGetPayload(t *testing.T) { // Put the 10th block's tx in the pool and produce a new block txs := blocks[9].Transactions() - ethservice.TxPool().Add(txs, true, false) + ethservice.TxPool().Add(txs, true, true) blockParams := engine.PayloadAttributes{ Timestamp: blocks[8].Time() + 5, } @@ -310,13 +310,13 @@ func TestEth2NewBlock(t *testing.T) { statedb, _ := ethservice.BlockChain().StateAt(parent.Root()) nonce := statedb.GetNonce(testAddr) tx, _ := types.SignTx(types.NewContractCreation(nonce, new(big.Int), 1000000, big.NewInt(2*params.InitialBaseFee), logCode), types.LatestSigner(ethservice.BlockChain().Config()), testKey) - ethservice.TxPool().Add([]*types.Transaction{tx}, true, false) + ethservice.TxPool().Add([]*types.Transaction{tx}, true, true) execData, err := assembleWithTransactions(api, parent.Hash(), &engine.PayloadAttributes{ Timestamp: parent.Time() + 5, }, 1) if err != nil { - t.Fatalf("Failed to create the executable data %v", err) + t.Fatalf("Failed to create the executable data, block %d: %v", i, err) } block, err := engine.ExecutableDataToBlock(*execData, nil, nil) if err != nil { diff --git a/triedb/pathdb/database.go b/triedb/pathdb/database.go index 2e7c662804..18f2eeef00 100644 --- a/triedb/pathdb/database.go +++ b/triedb/pathdb/database.go @@ -293,6 +293,7 @@ func (db *Database) Enable(root common.Hash) error { // Ensure the provided state root matches the stored one. root = types.TrieRootHash(root) _, stored := rawdb.ReadAccountTrieNode(db.diskdb, nil) + stored = types.TrieRootHash(stored) if stored != root { return fmt.Errorf("state root mismatch: stored %x, synced %x", stored, root) } diff --git a/triedb/pathdb/database_test.go b/triedb/pathdb/database_test.go index 30edef2760..29de534589 100644 --- a/triedb/pathdb/database_test.go +++ b/triedb/pathdb/database_test.go @@ -476,13 +476,13 @@ func TestDisable(t *testing.T) { _, stored := rawdb.ReadAccountTrieNode(tester.db.diskdb, nil) if err := tester.db.Disable(); err != nil { - t.Fatal("Failed to deactivate database") + t.Fatalf("Failed to deactivate database: %v", err) } if err := tester.db.Enable(types.EmptyRootHash); err == nil { - t.Fatalf("Invalid activation should be rejected") + t.Fatal("Invalid activation should be rejected") } if err := tester.db.Enable(stored); err != nil { - t.Fatal("Failed to activate database") + t.Fatalf("Failed to activate database: %v", err) } // Ensure journal is deleted from disk From 0e380ddaf7d9ccba87d3a3688a3fb419b562451c Mon Sep 17 00:00:00 2001 From: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com> Date: Tue, 23 Apr 2024 18:06:25 +0800 Subject: [PATCH 02/15] miner: fix typos (#29625) --- miner/worker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/miner/worker.go b/miner/worker.go index 4924952478..5dc3e2056b 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -339,7 +339,7 @@ func (miner *Miner) commitTransactions(env *environment, plainTxs, blobTxs *tran continue } // Error may be ignored here. The error has already been checked - // during transaction acceptance is the transaction pool. + // during transaction acceptance in the transaction pool. from, _ := types.Sender(env.signer, tx) // Check whether the tx is replay protected. If we're not in the EIP155 hf From 709e0b399712f113a907936b9f73da8c33afd3f1 Mon Sep 17 00:00:00 2001 From: Aaron Chen Date: Tue, 23 Apr 2024 18:08:02 +0800 Subject: [PATCH 03/15] metrics: remove librato (#29624) --- metrics/README.md | 19 --- metrics/librato/client.go | 104 --------------- metrics/librato/librato.go | 254 ------------------------------------- 3 files changed, 377 deletions(-) delete mode 100644 metrics/librato/client.go delete mode 100644 metrics/librato/librato.go diff --git a/metrics/README.md b/metrics/README.md index e2d7945008..85b119470a 100644 --- a/metrics/README.md +++ b/metrics/README.md @@ -100,24 +100,6 @@ go influxdb.InfluxDB(metrics.DefaultRegistry, ) ``` -Periodically upload every metric to Librato using the [Librato client](https://github.com/mihasya/go-metrics-librato): - -**Note**: the client included with this repository under the `librato` package -has been deprecated and moved to the repository linked above. - -```go -import "github.com/mihasya/go-metrics-librato" - -go librato.Librato(metrics.DefaultRegistry, - 10e9, // interval - "example@example.com", // account owner email address - "token", // Librato API token - "hostname", // source - []float64{0.95}, // percentiles to send - time.Millisecond, // time unit -) -``` - Periodically emit every metric to StatHat: ```go @@ -157,7 +139,6 @@ Publishing Metrics Clients are available for the following destinations: -* Librato - https://github.com/mihasya/go-metrics-librato * Graphite - https://github.com/cyberdelia/go-metrics-graphite * InfluxDB - https://github.com/vrischmann/go-metrics-influxdb * Ganglia - https://github.com/appscode/metlia diff --git a/metrics/librato/client.go b/metrics/librato/client.go deleted file mode 100644 index f1b9e1e916..0000000000 --- a/metrics/librato/client.go +++ /dev/null @@ -1,104 +0,0 @@ -package librato - -import ( - "bytes" - "encoding/json" - "fmt" - "io" - "net/http" -) - -const Operations = "operations" -const OperationsShort = "ops" - -type LibratoClient struct { - Email, Token string -} - -// property strings -const ( - // display attributes - Color = "color" - DisplayMax = "display_max" - DisplayMin = "display_min" - DisplayUnitsLong = "display_units_long" - DisplayUnitsShort = "display_units_short" - DisplayStacked = "display_stacked" - DisplayTransform = "display_transform" - // special gauge display attributes - SummarizeFunction = "summarize_function" - Aggregate = "aggregate" - - // metric keys - Name = "name" - Period = "period" - Description = "description" - DisplayName = "display_name" - Attributes = "attributes" - - // measurement keys - MeasureTime = "measure_time" - Source = "source" - Value = "value" - - // special gauge keys - Count = "count" - Sum = "sum" - Max = "max" - Min = "min" - SumSquares = "sum_squares" - - // batch keys - Counters = "counters" - Gauges = "gauges" - - MetricsPostUrl = "https://metrics-api.librato.com/v1/metrics" -) - -type Measurement map[string]interface{} -type Metric map[string]interface{} - -type Batch struct { - Gauges []Measurement `json:"gauges,omitempty"` - Counters []Measurement `json:"counters,omitempty"` - MeasureTime int64 `json:"measure_time"` - Source string `json:"source"` -} - -func (c *LibratoClient) PostMetrics(batch Batch) (err error) { - var ( - js []byte - req *http.Request - resp *http.Response - ) - - if len(batch.Counters) == 0 && len(batch.Gauges) == 0 { - return nil - } - - if js, err = json.Marshal(batch); err != nil { - return - } - - if req, err = http.NewRequest(http.MethodPost, MetricsPostUrl, bytes.NewBuffer(js)); err != nil { - return - } - - req.Header.Set("Content-Type", "application/json") - req.SetBasicAuth(c.Email, c.Token) - - resp, err = http.DefaultClient.Do(req) - if err != nil { - return - } - defer resp.Body.Close() - - if resp.StatusCode != http.StatusOK { - var body []byte - if body, err = io.ReadAll(resp.Body); err != nil { - body = []byte(fmt.Sprintf("(could not fetch response body for error: %s)", err)) - } - err = fmt.Errorf("unable to post to Librato: %d %s %s", resp.StatusCode, resp.Status, string(body)) - } - return -} diff --git a/metrics/librato/librato.go b/metrics/librato/librato.go deleted file mode 100644 index a86f758637..0000000000 --- a/metrics/librato/librato.go +++ /dev/null @@ -1,254 +0,0 @@ -package librato - -import ( - "fmt" - "log" - "math" - "regexp" - "time" - - "github.com/ethereum/go-ethereum/metrics" -) - -// a regexp for extracting the unit from time.Duration.String -var unitRegexp = regexp.MustCompile(`[^\\d]+$`) - -// a helper that turns a time.Duration into librato display attributes for timer metrics -func translateTimerAttributes(d time.Duration) (attrs map[string]interface{}) { - attrs = make(map[string]interface{}) - attrs[DisplayTransform] = fmt.Sprintf("x/%d", int64(d)) - attrs[DisplayUnitsShort] = string(unitRegexp.Find([]byte(d.String()))) - return -} - -type Reporter struct { - Email, Token string - Namespace string - Source string - Interval time.Duration - Registry metrics.Registry - Percentiles []float64 // percentiles to report on histogram metrics - TimerAttributes map[string]interface{} // units in which timers will be displayed - intervalSec int64 -} - -func NewReporter(r metrics.Registry, d time.Duration, e string, t string, s string, p []float64, u time.Duration) *Reporter { - return &Reporter{e, t, "", s, d, r, p, translateTimerAttributes(u), int64(d / time.Second)} -} - -func Librato(r metrics.Registry, d time.Duration, e string, t string, s string, p []float64, u time.Duration) { - NewReporter(r, d, e, t, s, p, u).Run() -} - -func (rep *Reporter) Run() { - log.Printf("WARNING: This client has been DEPRECATED! It has been moved to https://github.com/mihasya/go-metrics-librato and will be removed from rcrowley/go-metrics on August 5th 2015") - ticker := time.NewTicker(rep.Interval) - defer ticker.Stop() - metricsApi := &LibratoClient{rep.Email, rep.Token} - for now := range ticker.C { - var metrics Batch - var err error - if metrics, err = rep.BuildRequest(now, rep.Registry); err != nil { - log.Printf("ERROR constructing librato request body %s", err) - continue - } - if err := metricsApi.PostMetrics(metrics); err != nil { - log.Printf("ERROR sending metrics to librato %s", err) - continue - } - } -} - -// calculate sum of squares from data provided by metrics.Histogram -// see http://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods -func sumSquares(icount int64, mean, stDev float64) float64 { - count := float64(icount) - sumSquared := math.Pow(count*mean, 2) - sumSquares := math.Pow(count*stDev, 2) + sumSquared/count - if math.IsNaN(sumSquares) { - return 0.0 - } - return sumSquares -} -func sumSquaresTimer(t metrics.TimerSnapshot) float64 { - count := float64(t.Count()) - sumSquared := math.Pow(count*t.Mean(), 2) - sumSquares := math.Pow(count*t.StdDev(), 2) + sumSquared/count - if math.IsNaN(sumSquares) { - return 0.0 - } - return sumSquares -} - -func (rep *Reporter) BuildRequest(now time.Time, r metrics.Registry) (snapshot Batch, err error) { - snapshot = Batch{ - // coerce timestamps to a stepping fn so that they line up in Librato graphs - MeasureTime: (now.Unix() / rep.intervalSec) * rep.intervalSec, - Source: rep.Source, - } - snapshot.Gauges = make([]Measurement, 0) - snapshot.Counters = make([]Measurement, 0) - histogramGaugeCount := 1 + len(rep.Percentiles) - r.Each(func(name string, metric interface{}) { - if rep.Namespace != "" { - name = fmt.Sprintf("%s.%s", rep.Namespace, name) - } - measurement := Measurement{} - measurement[Period] = rep.Interval.Seconds() - switch m := metric.(type) { - case metrics.Counter: - ms := m.Snapshot() - if ms.Count() > 0 { - measurement[Name] = fmt.Sprintf("%s.%s", name, "count") - measurement[Value] = float64(ms.Count()) - measurement[Attributes] = map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - } - snapshot.Counters = append(snapshot.Counters, measurement) - } - case metrics.CounterFloat64: - if count := m.Snapshot().Count(); count > 0 { - measurement[Name] = fmt.Sprintf("%s.%s", name, "count") - measurement[Value] = count - measurement[Attributes] = map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - } - snapshot.Counters = append(snapshot.Counters, measurement) - } - case metrics.Gauge: - measurement[Name] = name - measurement[Value] = float64(m.Snapshot().Value()) - snapshot.Gauges = append(snapshot.Gauges, measurement) - case metrics.GaugeFloat64: - measurement[Name] = name - measurement[Value] = m.Snapshot().Value() - snapshot.Gauges = append(snapshot.Gauges, measurement) - case metrics.GaugeInfo: - measurement[Name] = name - measurement[Value] = m.Snapshot().Value() - snapshot.Gauges = append(snapshot.Gauges, measurement) - case metrics.Histogram: - ms := m.Snapshot() - if ms.Count() > 0 { - gauges := make([]Measurement, histogramGaugeCount) - measurement[Name] = fmt.Sprintf("%s.%s", name, "hist") - measurement[Count] = uint64(ms.Count()) - measurement[Max] = float64(ms.Max()) - measurement[Min] = float64(ms.Min()) - measurement[Sum] = float64(ms.Sum()) - measurement[SumSquares] = sumSquares(ms.Count(), ms.Mean(), ms.StdDev()) - gauges[0] = measurement - for i, p := range rep.Percentiles { - gauges[i+1] = Measurement{ - Name: fmt.Sprintf("%s.%.2f", measurement[Name], p), - Value: ms.Percentile(p), - Period: measurement[Period], - } - } - snapshot.Gauges = append(snapshot.Gauges, gauges...) - } - case metrics.Meter: - ms := m.Snapshot() - measurement[Name] = name - measurement[Value] = float64(ms.Count()) - snapshot.Counters = append(snapshot.Counters, measurement) - snapshot.Gauges = append(snapshot.Gauges, - Measurement{ - Name: fmt.Sprintf("%s.%s", name, "1min"), - Value: ms.Rate1(), - Period: int64(rep.Interval.Seconds()), - Attributes: map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - }, - }, - Measurement{ - Name: fmt.Sprintf("%s.%s", name, "5min"), - Value: ms.Rate5(), - Period: int64(rep.Interval.Seconds()), - Attributes: map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - }, - }, - Measurement{ - Name: fmt.Sprintf("%s.%s", name, "15min"), - Value: ms.Rate15(), - Period: int64(rep.Interval.Seconds()), - Attributes: map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - }, - }, - ) - case metrics.Timer: - ms := m.Snapshot() - measurement[Name] = name - measurement[Value] = float64(ms.Count()) - snapshot.Counters = append(snapshot.Counters, measurement) - if ms.Count() > 0 { - libratoName := fmt.Sprintf("%s.%s", name, "timer.mean") - gauges := make([]Measurement, histogramGaugeCount) - gauges[0] = Measurement{ - Name: libratoName, - Count: uint64(ms.Count()), - Sum: ms.Mean() * float64(ms.Count()), - Max: float64(ms.Max()), - Min: float64(ms.Min()), - SumSquares: sumSquaresTimer(ms), - Period: int64(rep.Interval.Seconds()), - Attributes: rep.TimerAttributes, - } - for i, p := range rep.Percentiles { - gauges[i+1] = Measurement{ - Name: fmt.Sprintf("%s.timer.%2.0f", name, p*100), - Value: ms.Percentile(p), - Period: int64(rep.Interval.Seconds()), - Attributes: rep.TimerAttributes, - } - } - snapshot.Gauges = append(snapshot.Gauges, gauges...) - snapshot.Gauges = append(snapshot.Gauges, - Measurement{ - Name: fmt.Sprintf("%s.%s", name, "rate.1min"), - Value: ms.Rate1(), - Period: int64(rep.Interval.Seconds()), - Attributes: map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - }, - }, - Measurement{ - Name: fmt.Sprintf("%s.%s", name, "rate.5min"), - Value: ms.Rate5(), - Period: int64(rep.Interval.Seconds()), - Attributes: map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - }, - }, - Measurement{ - Name: fmt.Sprintf("%s.%s", name, "rate.15min"), - Value: ms.Rate15(), - Period: int64(rep.Interval.Seconds()), - Attributes: map[string]interface{}{ - DisplayUnitsLong: Operations, - DisplayUnitsShort: OperationsShort, - DisplayMin: "0", - }, - }, - ) - } - } - }) - return -} From b2b0e1da8cac279bf0466885d1abdc5d93402f41 Mon Sep 17 00:00:00 2001 From: haoran <159284258+hr98w@users.noreply.github.com> Date: Tue, 23 Apr 2024 18:09:42 +0800 Subject: [PATCH 04/15] all: fix various typos (#29600) * core: fix typo * rpc: fix typo * snap: fix typo * trie: fix typo * main: fix typo * abi: fix typo * main: fix field comment for basicOp --- accounts/abi/type_test.go | 2 +- core/blockchain_insert.go | 2 +- core/state/iterator.go | 2 +- core/state/snapshot/context.go | 2 +- core/state/snapshot/iterator_binary.go | 4 ++-- core/state/snapshot/iterator_fast.go | 2 +- core/state/snapshot/snapshot.go | 2 +- core/state/state_object.go | 2 +- eth/protocols/snap/gentrie.go | 2 +- rlp/rlpgen/gen.go | 2 +- rpc/subscription.go | 2 +- rpc/types.go | 2 +- trie/errors.go | 2 +- trie/proof.go | 2 +- 14 files changed, 15 insertions(+), 15 deletions(-) diff --git a/accounts/abi/type_test.go b/accounts/abi/type_test.go index ae69872ad8..95922548c4 100644 --- a/accounts/abi/type_test.go +++ b/accounts/abi/type_test.go @@ -25,7 +25,7 @@ import ( "github.com/ethereum/go-ethereum/common" ) -// typeWithoutStringer is a alias for the Type type which simply doesn't implement +// typeWithoutStringer is an alias for the Type type which simply doesn't implement // the stringer interface to allow printing type details in the tests below. type typeWithoutStringer Type diff --git a/core/blockchain_insert.go b/core/blockchain_insert.go index c7c4c4bfea..49e913aada 100644 --- a/core/blockchain_insert.go +++ b/core/blockchain_insert.go @@ -170,7 +170,7 @@ func (it *insertIterator) current() *types.Header { return it.chain[it.index].Header() } -// first returns the first block in the it. +// first returns the first block in it. func (it *insertIterator) first() *types.Block { return it.chain[0] } diff --git a/core/state/iterator.go b/core/state/iterator.go index dc84ce689b..83c552ca1a 100644 --- a/core/state/iterator.go +++ b/core/state/iterator.go @@ -46,7 +46,7 @@ type nodeIterator struct { Error error // Failure set in case of an internal error in the iterator } -// newNodeIterator creates an post-order state node iterator. +// newNodeIterator creates a post-order state node iterator. func newNodeIterator(state *StateDB) *nodeIterator { return &nodeIterator{ state: state, diff --git a/core/state/snapshot/context.go b/core/state/snapshot/context.go index 67d7e41a03..8a19960501 100644 --- a/core/state/snapshot/context.go +++ b/core/state/snapshot/context.go @@ -46,7 +46,7 @@ type generatorStats struct { storage common.StorageSize // Total account and storage slot size(generation or recovery) } -// Log creates an contextual log with the given message and the context pulled +// Log creates a contextual log with the given message and the context pulled // from the internally maintained statistics. func (gs *generatorStats) Log(msg string, root common.Hash, marker []byte) { var ctx []interface{} diff --git a/core/state/snapshot/iterator_binary.go b/core/state/snapshot/iterator_binary.go index 22184b2545..edf471213f 100644 --- a/core/state/snapshot/iterator_binary.go +++ b/core/state/snapshot/iterator_binary.go @@ -68,7 +68,7 @@ func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { parent, ok := dl.parent.(*diffLayer) if !ok { // If the storage in this layer is already destructed, discard all - // deeper layers but still return an valid single-branch iterator. + // deeper layers but still return a valid single-branch iterator. a, destructed := dl.StorageIterator(account, common.Hash{}) if destructed { l := &binaryIterator{ @@ -92,7 +92,7 @@ func (dl *diffLayer) initBinaryStorageIterator(account common.Hash) Iterator { return l } // If the storage in this layer is already destructed, discard all - // deeper layers but still return an valid single-branch iterator. + // deeper layers but still return a valid single-branch iterator. a, destructed := dl.StorageIterator(account, common.Hash{}) if destructed { l := &binaryIterator{ diff --git a/core/state/snapshot/iterator_fast.go b/core/state/snapshot/iterator_fast.go index 9b018bac56..fa0daea7ba 100644 --- a/core/state/snapshot/iterator_fast.go +++ b/core/state/snapshot/iterator_fast.go @@ -25,7 +25,7 @@ import ( "github.com/ethereum/go-ethereum/common" ) -// weightedIterator is a iterator with an assigned weight. It is used to prioritise +// weightedIterator is an iterator with an assigned weight. It is used to prioritise // which account or storage slot is the correct one if multiple iterators find the // same one (modified in multiple consecutive blocks). type weightedIterator struct { diff --git a/core/state/snapshot/snapshot.go b/core/state/snapshot/snapshot.go index 5c38cb7252..89a4c16c20 100644 --- a/core/state/snapshot/snapshot.go +++ b/core/state/snapshot/snapshot.go @@ -835,7 +835,7 @@ func (t *Tree) disklayer() *diskLayer { } } -// diskRoot is a internal helper function to return the disk layer root. +// diskRoot is an internal helper function to return the disk layer root. // The lock of snapTree is assumed to be held already. func (t *Tree) diskRoot() common.Hash { disklayer := t.disklayer() diff --git a/core/state/state_object.go b/core/state/state_object.go index 1aa7946fd0..db3c32f2f2 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -376,7 +376,7 @@ func (s *stateObject) updateTrie() (Trie, error) { // new storage trie root. func (s *stateObject) updateRoot() { // Flush cached storage mutations into trie, short circuit if any error - // is occurred or there is not change in the trie. + // is occurred or there is no change in the trie. tr, err := s.updateTrie() if err != nil || tr == nil { return diff --git a/eth/protocols/snap/gentrie.go b/eth/protocols/snap/gentrie.go index 8ef1a00753..81c2640b62 100644 --- a/eth/protocols/snap/gentrie.go +++ b/eth/protocols/snap/gentrie.go @@ -132,7 +132,7 @@ func (t *pathTrie) onTrieNode(path []byte, hash common.Hash, blob []byte) { // // The extension node is detected if its path is the prefix of last committed // one and path gap is larger than one. If the path gap is only one byte, - // the current node could either be a full node, or a extension with single + // the current node could either be a full node, or an extension with single // byte key. In either case, no gaps will be left in the path. if t.last != nil && bytes.HasPrefix(t.last, path) && len(t.last)-len(path) > 1 { for i := len(path) + 1; i < len(t.last); i++ { diff --git a/rlp/rlpgen/gen.go b/rlp/rlpgen/gen.go index 0c65864826..ff39874737 100644 --- a/rlp/rlpgen/gen.go +++ b/rlp/rlpgen/gen.go @@ -158,7 +158,7 @@ type op interface { // basicOp handles basic types bool, uint*, string. type basicOp struct { typ types.Type - writeMethod string // calle write the value + writeMethod string // EncoderBuffer writer method name writeArgType types.Type // parameter type of writeMethod decMethod string decResultType types.Type // return type of decMethod diff --git a/rpc/subscription.go b/rpc/subscription.go index d3dff32a27..d77c655bf9 100644 --- a/rpc/subscription.go +++ b/rpc/subscription.go @@ -97,7 +97,7 @@ func NotifierFromContext(ctx context.Context) (*Notifier, bool) { return n, ok } -// Notifier is tied to a RPC connection that supports subscriptions. +// Notifier is tied to an RPC connection that supports subscriptions. // Server callbacks use the notifier to send notifications. type Notifier struct { h *handler diff --git a/rpc/types.go b/rpc/types.go index d124081786..2e53174b87 100644 --- a/rpc/types.go +++ b/rpc/types.go @@ -38,7 +38,7 @@ type API struct { } // ServerCodec implements reading, parsing and writing RPC messages for the server side of -// a RPC session. Implementations must be go-routine safe since the codec can be called in +// an RPC session. Implementations must be go-routine safe since the codec can be called in // multiple go-routines concurrently. type ServerCodec interface { peerInfo() PeerInfo diff --git a/trie/errors.go b/trie/errors.go index 7be7041c7f..ce5cb13423 100644 --- a/trie/errors.go +++ b/trie/errors.go @@ -23,7 +23,7 @@ import ( "github.com/ethereum/go-ethereum/common" ) -// ErrCommitted is returned when a already committed trie is requested for usage. +// ErrCommitted is returned when an already committed trie is requested for usage. // The potential usages can be `Get`, `Update`, `Delete`, `NodeIterator`, `Prove` // and so on. var ErrCommitted = errors.New("trie is already committed") diff --git a/trie/proof.go b/trie/proof.go index fd892fb4be..a39d6b4ea3 100644 --- a/trie/proof.go +++ b/trie/proof.go @@ -372,7 +372,7 @@ func unset(parent node, child node, key []byte, pos int, removeLeft bool) error return unset(cld, cld.Children[key[pos]], key, pos+1, removeLeft) case *shortNode: if len(key[pos:]) < len(cld.Key) || !bytes.Equal(cld.Key, key[pos:pos+len(cld.Key)]) { - // Find the fork point, it's an non-existent branch. + // Find the fork point, it's a non-existent branch. if removeLeft { if bytes.Compare(cld.Key, key[pos:]) < 0 { // The key of fork shortnode is less than the path From 256d4b099cf540ba99181d6e746d4a1eaebef054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felf=C3=B6ldi=20Zsolt?= Date: Tue, 23 Apr 2024 13:31:32 +0200 Subject: [PATCH 05/15] beacon/light: request finality update explicitly when necessary (#29567) This PR adds an extra mechanism to sync.HeadSync that tries to retrieve the latest finality update from every server each time it sends an optimistic update in a new epoch (unless we already have a validated finality update attested in the same epoch). Note that this is not necessary and does not happen if the new finality update is delivered before the optimistic update. The spec only mandates light_client_finality_update events when a new epoch is finalized. If the chain does not finalize for a while then we might need an explicit request that returns a finality proof that proves the same finality epoch from the latest attested epoch. --- beacon/blsync/block_sync.go | 18 ++--- beacon/blsync/block_sync_test.go | 8 +- beacon/blsync/engineclient.go | 3 + beacon/light/api/api_server.go | 16 ++-- beacon/light/api/light_api.go | 77 ++++++++++++------ beacon/light/head_tracker.go | 51 +++++++----- beacon/light/sync/head_sync.go | 110 ++++++++++++++++---------- beacon/light/sync/head_sync_test.go | 78 ++++++++++++------ beacon/light/sync/test_helpers.go | 27 ++++--- beacon/light/sync/types.go | 7 +- beacon/light/sync/update_sync.go | 10 ++- beacon/light/sync/update_sync_test.go | 12 +-- beacon/types/exec_payload.go | 5 +- beacon/types/light_sync.go | 53 ++++++++++++- 14 files changed, 316 insertions(+), 159 deletions(-) diff --git a/beacon/blsync/block_sync.go b/beacon/blsync/block_sync.go index 3ab156354d..ff689a922f 100755 --- a/beacon/blsync/block_sync.go +++ b/beacon/blsync/block_sync.go @@ -41,7 +41,7 @@ type beaconBlockSync struct { type headTracker interface { PrefetchHead() types.HeadInfo - ValidatedHead() (types.SignedHeader, bool) + ValidatedOptimistic() (types.OptimisticUpdate, bool) ValidatedFinality() (types.FinalityUpdate, bool) } @@ -66,6 +66,7 @@ func (s *beaconBlockSync) Process(requester request.Requester, events []request. case request.EvResponse, request.EvFail, request.EvTimeout: sid, req, resp := event.RequestInfo() blockRoot := common.Hash(req.(sync.ReqBeaconBlock)) + log.Debug("Beacon block event", "type", event.Type.Name, "hash", blockRoot) if resp != nil { s.recentBlocks.Add(blockRoot, resp.(*types.BeaconBlock)) } @@ -80,8 +81,8 @@ func (s *beaconBlockSync) Process(requester request.Requester, events []request. } s.updateEventFeed() // request validated head block if unavailable and not yet requested - if vh, ok := s.headTracker.ValidatedHead(); ok { - s.tryRequestBlock(requester, vh.Header.Hash(), false) + if vh, ok := s.headTracker.ValidatedOptimistic(); ok { + s.tryRequestBlock(requester, vh.Attested.Hash(), false) } // request prefetch head if the given server has announced it if prefetchHead := s.headTracker.PrefetchHead().BlockRoot; prefetchHead != (common.Hash{}) { @@ -114,12 +115,12 @@ func blockHeadInfo(block *types.BeaconBlock) types.HeadInfo { } func (s *beaconBlockSync) updateEventFeed() { - head, ok := s.headTracker.ValidatedHead() + optimistic, ok := s.headTracker.ValidatedOptimistic() if !ok { return } - validatedHead := head.Header.Hash() + validatedHead := optimistic.Attested.Hash() headBlock, ok := s.recentBlocks.Get(validatedHead) if !ok { return @@ -127,7 +128,7 @@ func (s *beaconBlockSync) updateEventFeed() { var finalizedHash common.Hash if finality, ok := s.headTracker.ValidatedFinality(); ok { - he := head.Header.Epoch() + he := optimistic.Attested.Epoch() fe := finality.Attested.Header.Epoch() switch { case he == fe: @@ -135,10 +136,9 @@ func (s *beaconBlockSync) updateEventFeed() { case he < fe: return case he == fe+1: - parent, ok := s.recentBlocks.Get(head.Header.ParentRoot) + parent, ok := s.recentBlocks.Get(optimistic.Attested.ParentRoot) if !ok || parent.Slot()/params.EpochLength == fe { return // head is at first slot of next epoch, wait for finality update - //TODO: try to fetch finality update directly if subscription does not deliver } } } @@ -156,7 +156,7 @@ func (s *beaconBlockSync) updateEventFeed() { return } s.chainHeadFeed.Send(types.ChainHeadEvent{ - BeaconHead: head.Header, + BeaconHead: optimistic.Attested.Header, Block: execBlock, Finalized: finalizedHash, }) diff --git a/beacon/blsync/block_sync_test.go b/beacon/blsync/block_sync_test.go index 0525e95a89..3d3b9e5e8d 100644 --- a/beacon/blsync/block_sync_test.go +++ b/beacon/blsync/block_sync_test.go @@ -140,8 +140,12 @@ func (h *testHeadTracker) PrefetchHead() types.HeadInfo { return h.prefetch } -func (h *testHeadTracker) ValidatedHead() (types.SignedHeader, bool) { - return h.validated, h.validated.Header != (types.Header{}) +func (h *testHeadTracker) ValidatedOptimistic() (types.OptimisticUpdate, bool) { + return types.OptimisticUpdate{ + Attested: types.HeaderWithExecProof{Header: h.validated.Header}, + Signature: h.validated.Signature, + SignatureSlot: h.validated.SignatureSlot, + }, h.validated.Header != (types.Header{}) } // TODO add test case for finality diff --git a/beacon/blsync/engineclient.go b/beacon/blsync/engineclient.go index 5a2d292a7d..97ef6f5cb8 100644 --- a/beacon/blsync/engineclient.go +++ b/beacon/blsync/engineclient.go @@ -62,6 +62,7 @@ func (ec *engineClient) updateLoop(headCh <-chan types.ChainHeadEvent) { for { select { case <-ec.rootCtx.Done(): + log.Debug("Stopping engine API update loop") return case event := <-headCh: @@ -73,12 +74,14 @@ func (ec *engineClient) updateLoop(headCh <-chan types.ChainHeadEvent) { fork := ec.config.ForkAtEpoch(event.BeaconHead.Epoch()) forkName := strings.ToLower(fork.Name) + log.Debug("Calling NewPayload", "number", event.Block.NumberU64(), "hash", event.Block.Hash()) if status, err := ec.callNewPayload(forkName, event); err == nil { log.Info("Successful NewPayload", "number", event.Block.NumberU64(), "hash", event.Block.Hash(), "status", status) } else { log.Error("Failed NewPayload", "number", event.Block.NumberU64(), "hash", event.Block.Hash(), "error", err) } + log.Debug("Calling ForkchoiceUpdated", "head", event.Block.Hash()) if status, err := ec.callForkchoiceUpdated(forkName, event); err == nil { log.Info("Successful ForkchoiceUpdated", "head", event.Block.Hash(), "status", status) } else { diff --git a/beacon/light/api/api_server.go b/beacon/light/api/api_server.go index 4b885cb8e1..2579854d82 100755 --- a/beacon/light/api/api_server.go +++ b/beacon/light/api/api_server.go @@ -46,13 +46,13 @@ func (s *ApiServer) Subscribe(eventCallback func(event request.Event)) { log.Debug("New head received", "slot", slot, "blockRoot", blockRoot) eventCallback(request.Event{Type: sync.EvNewHead, Data: types.HeadInfo{Slot: slot, BlockRoot: blockRoot}}) }, - OnSignedHead: func(head types.SignedHeader) { - log.Debug("New signed head received", "slot", head.Header.Slot, "blockRoot", head.Header.Hash(), "signerCount", head.Signature.SignerCount()) - eventCallback(request.Event{Type: sync.EvNewSignedHead, Data: head}) + OnOptimistic: func(update types.OptimisticUpdate) { + log.Debug("New optimistic update received", "slot", update.Attested.Slot, "blockRoot", update.Attested.Hash(), "signerCount", update.Signature.SignerCount()) + eventCallback(request.Event{Type: sync.EvNewOptimisticUpdate, Data: update}) }, - OnFinality: func(head types.FinalityUpdate) { - log.Debug("New finality update received", "slot", head.Attested.Slot, "blockRoot", head.Attested.Hash(), "signerCount", head.Signature.SignerCount()) - eventCallback(request.Event{Type: sync.EvNewFinalityUpdate, Data: head}) + OnFinality: func(update types.FinalityUpdate) { + log.Debug("New finality update received", "slot", update.Attested.Slot, "blockRoot", update.Attested.Hash(), "signerCount", update.Signature.SignerCount()) + eventCallback(request.Event{Type: sync.EvNewFinalityUpdate, Data: update}) }, OnError: func(err error) { log.Warn("Head event stream error", "err", err) @@ -83,6 +83,9 @@ func (s *ApiServer) SendRequest(id request.ID, req request.Request) { case sync.ReqBeaconBlock: log.Debug("Beacon API: requesting block", "reqid", id, "hash", common.Hash(data)) resp, err = s.api.GetBeaconBlock(common.Hash(data)) + case sync.ReqFinality: + log.Debug("Beacon API: requesting finality update") + resp, err = s.api.GetFinalityUpdate() default: } @@ -90,6 +93,7 @@ func (s *ApiServer) SendRequest(id request.ID, req request.Request) { log.Warn("Beacon API request failed", "type", reflect.TypeOf(req), "reqid", id, "err", err) s.eventCallback(request.Event{Type: request.EvFail, Data: request.RequestResponse{ID: id, Request: req}}) } else { + log.Debug("Beacon API request answered", "type", reflect.TypeOf(req), "reqid", id) s.eventCallback(request.Event{Type: request.EvResponse, Data: request.RequestResponse{ID: id, Request: req, Response: resp}}) } }() diff --git a/beacon/light/api/light_api.go b/beacon/light/api/light_api.go index 6892407caf..903db57344 100755 --- a/beacon/light/api/light_api.go +++ b/beacon/light/api/light_api.go @@ -32,6 +32,7 @@ import ( "github.com/ethereum/go-ethereum/beacon/types" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/ethereum/go-ethereum/log" ) var ( @@ -184,46 +185,56 @@ func (api *BeaconLightApi) GetBestUpdatesAndCommittees(firstPeriod, count uint64 return updates, committees, nil } -// GetOptimisticHeadUpdate fetches a signed header based on the latest available -// optimistic update. Note that the signature should be verified by the caller -// as its validity depends on the update chain. +// GetOptimisticUpdate fetches the latest available optimistic update. +// Note that the signature should be verified by the caller as its validity +// depends on the update chain. // // See data structure definition here: // https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md#lightclientoptimisticupdate -func (api *BeaconLightApi) GetOptimisticHeadUpdate() (types.SignedHeader, error) { +func (api *BeaconLightApi) GetOptimisticUpdate() (types.OptimisticUpdate, error) { resp, err := api.httpGet("/eth/v1/beacon/light_client/optimistic_update") if err != nil { - return types.SignedHeader{}, err + return types.OptimisticUpdate{}, err } - return decodeOptimisticHeadUpdate(resp) + return decodeOptimisticUpdate(resp) } -func decodeOptimisticHeadUpdate(enc []byte) (types.SignedHeader, error) { +func decodeOptimisticUpdate(enc []byte) (types.OptimisticUpdate, error) { var data struct { - Data struct { - Header jsonBeaconHeader `json:"attested_header"` - Aggregate types.SyncAggregate `json:"sync_aggregate"` - SignatureSlot common.Decimal `json:"signature_slot"` + Version string + Data struct { + Attested jsonHeaderWithExecProof `json:"attested_header"` + Aggregate types.SyncAggregate `json:"sync_aggregate"` + SignatureSlot common.Decimal `json:"signature_slot"` } `json:"data"` } if err := json.Unmarshal(enc, &data); err != nil { - return types.SignedHeader{}, err + return types.OptimisticUpdate{}, err } - if data.Data.Header.Beacon.StateRoot == (common.Hash{}) { + // Decode the execution payload headers. + attestedExecHeader, err := types.ExecutionHeaderFromJSON(data.Version, data.Data.Attested.Execution) + if err != nil { + return types.OptimisticUpdate{}, fmt.Errorf("invalid attested header: %v", err) + } + if data.Data.Attested.Beacon.StateRoot == (common.Hash{}) { // workaround for different event encoding format in Lodestar if err := json.Unmarshal(enc, &data.Data); err != nil { - return types.SignedHeader{}, err + return types.OptimisticUpdate{}, err } } if len(data.Data.Aggregate.Signers) != params.SyncCommitteeBitmaskSize { - return types.SignedHeader{}, errors.New("invalid sync_committee_bits length") + return types.OptimisticUpdate{}, errors.New("invalid sync_committee_bits length") } if len(data.Data.Aggregate.Signature) != params.BLSSignatureSize { - return types.SignedHeader{}, errors.New("invalid sync_committee_signature length") + return types.OptimisticUpdate{}, errors.New("invalid sync_committee_signature length") } - return types.SignedHeader{ - Header: data.Data.Header.Beacon, + return types.OptimisticUpdate{ + Attested: types.HeaderWithExecProof{ + Header: data.Data.Attested.Beacon, + PayloadHeader: attestedExecHeader, + PayloadBranch: data.Data.Attested.ExecutionBranch, + }, Signature: data.Data.Aggregate, SignatureSlot: uint64(data.Data.SignatureSlot), }, nil @@ -411,7 +422,7 @@ func decodeHeadEvent(enc []byte) (uint64, common.Hash, error) { type HeadEventListener struct { OnNewHead func(slot uint64, blockRoot common.Hash) - OnSignedHead func(head types.SignedHeader) + OnOptimistic func(head types.OptimisticUpdate) OnFinality func(head types.FinalityUpdate) OnError func(err error) } @@ -449,21 +460,35 @@ func (api *BeaconLightApi) StartHeadListener(listener HeadEventListener) func() defer wg.Done() // Request initial data. + log.Trace("Requesting initial head header") if head, _, _, err := api.GetHeader(common.Hash{}); err == nil { + log.Trace("Retrieved initial head header", "slot", head.Slot, "hash", head.Hash()) listener.OnNewHead(head.Slot, head.Hash()) + } else { + log.Debug("Failed to retrieve initial head header", "error", err) } - if signedHead, err := api.GetOptimisticHeadUpdate(); err == nil { - listener.OnSignedHead(signedHead) + log.Trace("Requesting initial optimistic update") + if optimisticUpdate, err := api.GetOptimisticUpdate(); err == nil { + log.Trace("Retrieved initial optimistic update", "slot", optimisticUpdate.Attested.Slot, "hash", optimisticUpdate.Attested.Hash()) + listener.OnOptimistic(optimisticUpdate) + } else { + log.Debug("Failed to retrieve initial optimistic update", "error", err) } + log.Trace("Requesting initial finality update") if finalityUpdate, err := api.GetFinalityUpdate(); err == nil { + log.Trace("Retrieved initial finality update", "slot", finalityUpdate.Finalized.Slot, "hash", finalityUpdate.Finalized.Hash()) listener.OnFinality(finalityUpdate) + } else { + log.Debug("Failed to retrieve initial finality update", "error", err) } + log.Trace("Starting event stream processing loop") // Receive the stream. var stream *eventsource.Stream select { case stream = <-streamCh: case <-ctx.Done(): + log.Trace("Stopping event stream processing loop") return } @@ -474,8 +499,10 @@ func (api *BeaconLightApi) StartHeadListener(listener HeadEventListener) func() case event, ok := <-stream.Events: if !ok { + log.Trace("Event stream closed") return } + log.Trace("New event received from event stream", "type", event.Event()) switch event.Event() { case "head": slot, blockRoot, err := decodeHeadEvent([]byte(event.Data())) @@ -485,9 +512,9 @@ func (api *BeaconLightApi) StartHeadListener(listener HeadEventListener) func() listener.OnError(fmt.Errorf("error decoding head event: %v", err)) } case "light_client_optimistic_update": - signedHead, err := decodeOptimisticHeadUpdate([]byte(event.Data())) + optimisticUpdate, err := decodeOptimisticUpdate([]byte(event.Data())) if err == nil { - listener.OnSignedHead(signedHead) + listener.OnOptimistic(optimisticUpdate) } else { listener.OnError(fmt.Errorf("error decoding optimistic update event: %v", err)) } @@ -521,7 +548,8 @@ func (api *BeaconLightApi) StartHeadListener(listener HeadEventListener) func() // established. It can only return nil when the context is canceled. func (api *BeaconLightApi) startEventStream(ctx context.Context, listener *HeadEventListener) *eventsource.Stream { for retry := true; retry; retry = ctxSleep(ctx, 5*time.Second) { - path := "/eth/v1/events?topics=head&topics=light_client_optimistic_update&topics=light_client_finality_update" + path := "/eth/v1/events?topics=head&topics=light_client_finality_update&topics=light_client_optimistic_update" + log.Trace("Sending event subscription request") req, err := http.NewRequestWithContext(ctx, "GET", api.url+path, nil) if err != nil { listener.OnError(fmt.Errorf("error creating event subscription request: %v", err)) @@ -535,6 +563,7 @@ func (api *BeaconLightApi) startEventStream(ctx context.Context, listener *HeadE listener.OnError(fmt.Errorf("error creating event subscription: %v", err)) continue } + log.Trace("Successfully created event stream") return stream } return nil diff --git a/beacon/light/head_tracker.go b/beacon/light/head_tracker.go index 6036322f01..7ef93fecce 100644 --- a/beacon/light/head_tracker.go +++ b/beacon/light/head_tracker.go @@ -29,15 +29,15 @@ import ( // which is the (not necessarily validated) head announced by the majority of // servers. type HeadTracker struct { - lock sync.RWMutex - committeeChain *CommitteeChain - minSignerCount int - signedHead types.SignedHeader - hasSignedHead bool - finalityUpdate types.FinalityUpdate - hasFinalityUpdate bool - prefetchHead types.HeadInfo - changeCounter uint64 + lock sync.RWMutex + committeeChain *CommitteeChain + minSignerCount int + optimisticUpdate types.OptimisticUpdate + hasOptimisticUpdate bool + finalityUpdate types.FinalityUpdate + hasFinalityUpdate bool + prefetchHead types.HeadInfo + changeCounter uint64 } // NewHeadTracker creates a new HeadTracker. @@ -48,15 +48,15 @@ func NewHeadTracker(committeeChain *CommitteeChain, minSignerCount int) *HeadTra } } -// ValidatedHead returns the latest validated head. -func (h *HeadTracker) ValidatedHead() (types.SignedHeader, bool) { +// ValidatedOptimistic returns the latest validated optimistic update. +func (h *HeadTracker) ValidatedOptimistic() (types.OptimisticUpdate, bool) { h.lock.RLock() defer h.lock.RUnlock() - return h.signedHead, h.hasSignedHead + return h.optimisticUpdate, h.hasOptimisticUpdate } -// ValidatedFinality returns the latest validated finality. +// ValidatedFinality returns the latest validated finality update. func (h *HeadTracker) ValidatedFinality() (types.FinalityUpdate, bool) { h.lock.RLock() defer h.lock.RUnlock() @@ -64,26 +64,36 @@ func (h *HeadTracker) ValidatedFinality() (types.FinalityUpdate, bool) { return h.finalityUpdate, h.hasFinalityUpdate } -// ValidateHead validates the given signed head. If the head is successfully validated -// and it is better than the old validated head (higher slot or same slot and more -// signers) then ValidatedHead is updated. The boolean return flag signals if -// ValidatedHead has been changed. -func (h *HeadTracker) ValidateHead(head types.SignedHeader) (bool, error) { +// ValidateOptimistic validates the given optimistic update. If the update is +// successfully validated and it is better than the old validated update (higher +// slot or same slot and more signers) then ValidatedOptimistic is updated. +// The boolean return flag signals if ValidatedOptimistic has been changed. +func (h *HeadTracker) ValidateOptimistic(update types.OptimisticUpdate) (bool, error) { h.lock.Lock() defer h.lock.Unlock() - replace, err := h.validate(head, h.signedHead) + if err := update.Validate(); err != nil { + return false, err + } + replace, err := h.validate(update.SignedHeader(), h.optimisticUpdate.SignedHeader()) if replace { - h.signedHead, h.hasSignedHead = head, true + h.optimisticUpdate, h.hasOptimisticUpdate = update, true h.changeCounter++ } return replace, err } +// ValidateFinality validates the given finality update. If the update is +// successfully validated and it is better than the old validated update (higher +// slot or same slot and more signers) then ValidatedFinality is updated. +// The boolean return flag signals if ValidatedFinality has been changed. func (h *HeadTracker) ValidateFinality(update types.FinalityUpdate) (bool, error) { h.lock.Lock() defer h.lock.Unlock() + if err := update.Validate(); err != nil { + return false, err + } replace, err := h.validate(update.SignedHeader(), h.finalityUpdate.SignedHeader()) if replace { h.finalityUpdate, h.hasFinalityUpdate = update, true @@ -142,6 +152,7 @@ func (h *HeadTracker) SetPrefetchHead(head types.HeadInfo) { h.changeCounter++ } +// ChangeCounter implements request.targetData func (h *HeadTracker) ChangeCounter() uint64 { h.lock.RLock() defer h.lock.RUnlock() diff --git a/beacon/light/sync/head_sync.go b/beacon/light/sync/head_sync.go index 5ccc2e18a2..dd05d39588 100644 --- a/beacon/light/sync/head_sync.go +++ b/beacon/light/sync/head_sync.go @@ -19,11 +19,13 @@ package sync import ( "github.com/ethereum/go-ethereum/beacon/light/request" "github.com/ethereum/go-ethereum/beacon/types" + "github.com/ethereum/go-ethereum/log" ) type headTracker interface { - ValidateHead(head types.SignedHeader) (bool, error) + ValidateOptimistic(update types.OptimisticUpdate) (bool, error) ValidateFinality(head types.FinalityUpdate) (bool, error) + ValidatedFinality() (types.FinalityUpdate, bool) SetPrefetchHead(head types.HeadInfo) } @@ -33,16 +35,17 @@ type headTracker interface { // It can also postpone the validation of the latest announced signed head // until the committee chain is synced up to at least the required period. type HeadSync struct { - headTracker headTracker - chain committeeChain - nextSyncPeriod uint64 - chainInit bool - unvalidatedHeads map[request.Server]types.SignedHeader - unvalidatedFinality map[request.Server]types.FinalityUpdate - serverHeads map[request.Server]types.HeadInfo - headServerCount map[types.HeadInfo]headServerCount - headCounter uint64 - prefetchHead types.HeadInfo + headTracker headTracker + chain committeeChain + nextSyncPeriod uint64 + chainInit bool + unvalidatedOptimistic map[request.Server]types.OptimisticUpdate + unvalidatedFinality map[request.Server]types.FinalityUpdate + serverHeads map[request.Server]types.HeadInfo + reqFinalityEpoch map[request.Server]uint64 // next epoch to request finality update + headServerCount map[types.HeadInfo]headServerCount + headCounter uint64 + prefetchHead types.HeadInfo } // headServerCount is associated with most recently seen head infos; it counts @@ -57,75 +60,98 @@ type headServerCount struct { // NewHeadSync creates a new HeadSync. func NewHeadSync(headTracker headTracker, chain committeeChain) *HeadSync { s := &HeadSync{ - headTracker: headTracker, - chain: chain, - unvalidatedHeads: make(map[request.Server]types.SignedHeader), - unvalidatedFinality: make(map[request.Server]types.FinalityUpdate), - serverHeads: make(map[request.Server]types.HeadInfo), - headServerCount: make(map[types.HeadInfo]headServerCount), + headTracker: headTracker, + chain: chain, + unvalidatedOptimistic: make(map[request.Server]types.OptimisticUpdate), + unvalidatedFinality: make(map[request.Server]types.FinalityUpdate), + serverHeads: make(map[request.Server]types.HeadInfo), + headServerCount: make(map[types.HeadInfo]headServerCount), + reqFinalityEpoch: make(map[request.Server]uint64), } return s } // Process implements request.Module. func (s *HeadSync) Process(requester request.Requester, events []request.Event) { + nextPeriod, chainInit := s.chain.NextSyncPeriod() + if nextPeriod != s.nextSyncPeriod || chainInit != s.chainInit { + s.nextSyncPeriod, s.chainInit = nextPeriod, chainInit + s.processUnvalidatedUpdates() + } + for _, event := range events { switch event.Type { case EvNewHead: s.setServerHead(event.Server, event.Data.(types.HeadInfo)) - case EvNewSignedHead: - s.newSignedHead(event.Server, event.Data.(types.SignedHeader)) + case EvNewOptimisticUpdate: + update := event.Data.(types.OptimisticUpdate) + s.newOptimisticUpdate(event.Server, update) + epoch := update.Attested.Epoch() + if epoch < s.reqFinalityEpoch[event.Server] { + continue + } + if finality, ok := s.headTracker.ValidatedFinality(); ok && finality.Attested.Header.Epoch() >= epoch { + continue + } + requester.Send(event.Server, ReqFinality{}) + s.reqFinalityEpoch[event.Server] = epoch + 1 case EvNewFinalityUpdate: s.newFinalityUpdate(event.Server, event.Data.(types.FinalityUpdate)) + case request.EvResponse: + _, _, resp := event.RequestInfo() + s.newFinalityUpdate(event.Server, resp.(types.FinalityUpdate)) case request.EvUnregistered: s.setServerHead(event.Server, types.HeadInfo{}) delete(s.serverHeads, event.Server) - delete(s.unvalidatedHeads, event.Server) + delete(s.unvalidatedOptimistic, event.Server) + delete(s.unvalidatedFinality, event.Server) } } - - nextPeriod, chainInit := s.chain.NextSyncPeriod() - if nextPeriod != s.nextSyncPeriod || chainInit != s.chainInit { - s.nextSyncPeriod, s.chainInit = nextPeriod, chainInit - s.processUnvalidated() - } } -// newSignedHead handles received signed head; either validates it if the chain -// is properly synced or stores it for further validation. -func (s *HeadSync) newSignedHead(server request.Server, signedHead types.SignedHeader) { - if !s.chainInit || types.SyncPeriod(signedHead.SignatureSlot) > s.nextSyncPeriod { - s.unvalidatedHeads[server] = signedHead +// newOptimisticUpdate handles received optimistic update; either validates it if +// the chain is properly synced or stores it for further validation. +func (s *HeadSync) newOptimisticUpdate(server request.Server, optimisticUpdate types.OptimisticUpdate) { + if !s.chainInit || types.SyncPeriod(optimisticUpdate.SignatureSlot) > s.nextSyncPeriod { + s.unvalidatedOptimistic[server] = optimisticUpdate return } - s.headTracker.ValidateHead(signedHead) + if _, err := s.headTracker.ValidateOptimistic(optimisticUpdate); err != nil { + log.Debug("Error validating optimistic update", "error", err) + } } -// newFinalityUpdate handles received finality update; either validates it if the chain -// is properly synced or stores it for further validation. +// newFinalityUpdate handles received finality update; either validates it if +// the chain is properly synced or stores it for further validation. func (s *HeadSync) newFinalityUpdate(server request.Server, finalityUpdate types.FinalityUpdate) { if !s.chainInit || types.SyncPeriod(finalityUpdate.SignatureSlot) > s.nextSyncPeriod { s.unvalidatedFinality[server] = finalityUpdate return } - s.headTracker.ValidateFinality(finalityUpdate) + if _, err := s.headTracker.ValidateFinality(finalityUpdate); err != nil { + log.Debug("Error validating finality update", "error", err) + } } -// processUnvalidated iterates the list of unvalidated heads and validates +// processUnvalidatedUpdates iterates the list of unvalidated updates and validates // those which can be validated. -func (s *HeadSync) processUnvalidated() { +func (s *HeadSync) processUnvalidatedUpdates() { if !s.chainInit { return } - for server, signedHead := range s.unvalidatedHeads { - if types.SyncPeriod(signedHead.SignatureSlot) <= s.nextSyncPeriod { - s.headTracker.ValidateHead(signedHead) - delete(s.unvalidatedHeads, server) + for server, optimisticUpdate := range s.unvalidatedOptimistic { + if types.SyncPeriod(optimisticUpdate.SignatureSlot) <= s.nextSyncPeriod { + if _, err := s.headTracker.ValidateOptimistic(optimisticUpdate); err != nil { + log.Debug("Error validating deferred optimistic update", "error", err) + } + delete(s.unvalidatedOptimistic, server) } } for server, finalityUpdate := range s.unvalidatedFinality { if types.SyncPeriod(finalityUpdate.SignatureSlot) <= s.nextSyncPeriod { - s.headTracker.ValidateFinality(finalityUpdate) + if _, err := s.headTracker.ValidateFinality(finalityUpdate); err != nil { + log.Debug("Error validating deferred finality update", "error", err) + } delete(s.unvalidatedFinality, server) } } diff --git a/beacon/light/sync/head_sync_test.go b/beacon/light/sync/head_sync_test.go index a2870b2732..cd7dacf7fe 100644 --- a/beacon/light/sync/head_sync_test.go +++ b/beacon/light/sync/head_sync_test.go @@ -19,6 +19,7 @@ package sync import ( "testing" + "github.com/ethereum/go-ethereum/beacon/light/request" "github.com/ethereum/go-ethereum/beacon/types" "github.com/ethereum/go-ethereum/common" ) @@ -28,6 +29,7 @@ var ( testServer2 = testServer("testServer2") testServer3 = testServer("testServer3") testServer4 = testServer("testServer4") + testServer5 = testServer("testServer5") testHead0 = types.HeadInfo{} testHead1 = types.HeadInfo{Slot: 123, BlockRoot: common.Hash{1}} @@ -35,13 +37,21 @@ var ( testHead3 = types.HeadInfo{Slot: 124, BlockRoot: common.Hash{3}} testHead4 = types.HeadInfo{Slot: 125, BlockRoot: common.Hash{4}} - testSHead1 = types.SignedHeader{SignatureSlot: 0x0124, Header: types.Header{Slot: 0x0123, StateRoot: common.Hash{1}}} - testSHead2 = types.SignedHeader{SignatureSlot: 0x2010, Header: types.Header{Slot: 0x200e, StateRoot: common.Hash{2}}} - // testSHead3 is at the end of period 1 but signed in period 2 - testSHead3 = types.SignedHeader{SignatureSlot: 0x4000, Header: types.Header{Slot: 0x3fff, StateRoot: common.Hash{3}}} - testSHead4 = types.SignedHeader{SignatureSlot: 0x6444, Header: types.Header{Slot: 0x6443, StateRoot: common.Hash{4}}} + testOptUpdate1 = types.OptimisticUpdate{SignatureSlot: 0x0124, Attested: types.HeaderWithExecProof{Header: types.Header{Slot: 0x0123, StateRoot: common.Hash{1}}}} + testOptUpdate2 = types.OptimisticUpdate{SignatureSlot: 0x2010, Attested: types.HeaderWithExecProof{Header: types.Header{Slot: 0x200e, StateRoot: common.Hash{2}}}} + // testOptUpdate3 is at the end of period 1 but signed in period 2 + testOptUpdate3 = types.OptimisticUpdate{SignatureSlot: 0x4000, Attested: types.HeaderWithExecProof{Header: types.Header{Slot: 0x3fff, StateRoot: common.Hash{3}}}} + testOptUpdate4 = types.OptimisticUpdate{SignatureSlot: 0x6444, Attested: types.HeaderWithExecProof{Header: types.Header{Slot: 0x6443, StateRoot: common.Hash{4}}}} ) +func finality(opt types.OptimisticUpdate) types.FinalityUpdate { + return types.FinalityUpdate{ + SignatureSlot: opt.SignatureSlot, + Attested: opt.Attested, + Finalized: types.HeaderWithExecProof{Header: types.Header{Slot: (opt.Attested.Header.Slot - 64) & uint64(0xffffffffffffffe0)}}, + } +} + type testServer string func (t testServer) Name() string { @@ -57,50 +67,66 @@ func TestValidatedHead(t *testing.T) { ht.ExpValidated(t, 0, nil) ts.AddServer(testServer1, 1) - ts.ServerEvent(EvNewSignedHead, testServer1, testSHead1) - ts.Run(1) + ts.ServerEvent(EvNewOptimisticUpdate, testServer1, testOptUpdate1) + ts.Run(1, testServer1, ReqFinality{}) // announced head should be queued because of uninitialized chain ht.ExpValidated(t, 1, nil) chain.SetNextSyncPeriod(0) // initialize chain ts.Run(2) // expect previously queued head to be validated - ht.ExpValidated(t, 2, []types.SignedHeader{testSHead1}) + ht.ExpValidated(t, 2, []types.OptimisticUpdate{testOptUpdate1}) chain.SetNextSyncPeriod(1) - ts.ServerEvent(EvNewSignedHead, testServer1, testSHead2) + ts.ServerEvent(EvNewFinalityUpdate, testServer1, finality(testOptUpdate2)) + ts.ServerEvent(EvNewOptimisticUpdate, testServer1, testOptUpdate2) ts.AddServer(testServer2, 1) - ts.ServerEvent(EvNewSignedHead, testServer2, testSHead2) + ts.ServerEvent(EvNewOptimisticUpdate, testServer2, testOptUpdate2) ts.Run(3) // expect both head announcements to be validated instantly - ht.ExpValidated(t, 3, []types.SignedHeader{testSHead2, testSHead2}) + ht.ExpValidated(t, 3, []types.OptimisticUpdate{testOptUpdate2, testOptUpdate2}) - ts.ServerEvent(EvNewSignedHead, testServer1, testSHead3) + ts.ServerEvent(EvNewOptimisticUpdate, testServer1, testOptUpdate3) ts.AddServer(testServer3, 1) - ts.ServerEvent(EvNewSignedHead, testServer3, testSHead4) - ts.Run(4) - // future period announced heads should be queued + ts.ServerEvent(EvNewOptimisticUpdate, testServer3, testOptUpdate4) + // finality should be requested from both servers + ts.Run(4, testServer1, ReqFinality{}, testServer3, ReqFinality{}) + // future period annonced heads should be queued ht.ExpValidated(t, 4, nil) chain.SetNextSyncPeriod(2) ts.Run(5) - // testSHead3 can be validated now but not testSHead4 - ht.ExpValidated(t, 5, []types.SignedHeader{testSHead3}) + // testOptUpdate3 can be validated now but not testOptUpdate4 + ht.ExpValidated(t, 5, []types.OptimisticUpdate{testOptUpdate3}) + + ts.AddServer(testServer4, 1) + ts.ServerEvent(EvNewOptimisticUpdate, testServer4, testOptUpdate3) + // new server joined with recent optimistic update but still no finality; should be requested + ts.Run(6, testServer4, ReqFinality{}) + ht.ExpValidated(t, 6, []types.OptimisticUpdate{testOptUpdate3}) + + ts.AddServer(testServer5, 1) + ts.RequestEvent(request.EvResponse, ts.Request(6, 1), finality(testOptUpdate3)) + ts.ServerEvent(EvNewOptimisticUpdate, testServer5, testOptUpdate3) + // finality update request answered; new server should not be requested + ts.Run(7) + ht.ExpValidated(t, 7, []types.OptimisticUpdate{testOptUpdate3}) // server 3 disconnected without proving period 3, its announced head should be dropped ts.RemoveServer(testServer3) - ts.Run(6) - ht.ExpValidated(t, 6, nil) + ts.Run(8) + ht.ExpValidated(t, 8, nil) chain.SetNextSyncPeriod(3) - ts.Run(7) - // testSHead4 could be validated now but it's not queued by any registered server - ht.ExpValidated(t, 7, nil) + ts.Run(9) + // testOptUpdate4 could be validated now but it's not queued by any registered server + ht.ExpValidated(t, 9, nil) - ts.ServerEvent(EvNewSignedHead, testServer2, testSHead4) - ts.Run(8) - // now testSHead4 should be validated - ht.ExpValidated(t, 8, []types.SignedHeader{testSHead4}) + ts.ServerEvent(EvNewFinalityUpdate, testServer2, finality(testOptUpdate4)) + ts.ServerEvent(EvNewOptimisticUpdate, testServer2, testOptUpdate4) + ts.Run(10) + // now testOptUpdate4 should be validated + ht.ExpValidated(t, 10, []types.OptimisticUpdate{testOptUpdate4}) } func TestPrefetchHead(t *testing.T) { diff --git a/beacon/light/sync/test_helpers.go b/beacon/light/sync/test_helpers.go index 9f57ceebe4..cfca8ad8a4 100644 --- a/beacon/light/sync/test_helpers.go +++ b/beacon/light/sync/test_helpers.go @@ -212,32 +212,37 @@ func (tc *TestCommitteeChain) ExpNextSyncPeriod(t *testing.T, expNsp uint64) { type TestHeadTracker struct { phead types.HeadInfo - validated []types.SignedHeader + validated []types.OptimisticUpdate + finality types.FinalityUpdate } -func (ht *TestHeadTracker) ValidateHead(head types.SignedHeader) (bool, error) { - ht.validated = append(ht.validated, head) +func (ht *TestHeadTracker) ValidateOptimistic(update types.OptimisticUpdate) (bool, error) { + ht.validated = append(ht.validated, update) return true, nil } -// TODO add test case for finality -func (ht *TestHeadTracker) ValidateFinality(head types.FinalityUpdate) (bool, error) { +func (ht *TestHeadTracker) ValidateFinality(update types.FinalityUpdate) (bool, error) { + ht.finality = update return true, nil } -func (ht *TestHeadTracker) ExpValidated(t *testing.T, tci int, expHeads []types.SignedHeader) { +func (ht *TestHeadTracker) ValidatedFinality() (types.FinalityUpdate, bool) { + return ht.finality, ht.finality.Attested.Header != (types.Header{}) +} + +func (ht *TestHeadTracker) ExpValidated(t *testing.T, tci int, expHeads []types.OptimisticUpdate) { for i, expHead := range expHeads { if i >= len(ht.validated) { - t.Errorf("Missing validated head in test case #%d index #%d (expected {slot %d blockRoot %x}, got none)", tci, i, expHead.Header.Slot, expHead.Header.Hash()) + t.Errorf("Missing validated head in test case #%d index #%d (expected {slot %d blockRoot %x}, got none)", tci, i, expHead.Attested.Header.Slot, expHead.Attested.Header.Hash()) continue } - if ht.validated[i] != expHead { - vhead := ht.validated[i].Header - t.Errorf("Wrong validated head in test case #%d index #%d (expected {slot %d blockRoot %x}, got {slot %d blockRoot %x})", tci, i, expHead.Header.Slot, expHead.Header.Hash(), vhead.Slot, vhead.Hash()) + if !reflect.DeepEqual(ht.validated[i], expHead) { + vhead := ht.validated[i].Attested.Header + t.Errorf("Wrong validated head in test case #%d index #%d (expected {slot %d blockRoot %x}, got {slot %d blockRoot %x})", tci, i, expHead.Attested.Header.Slot, expHead.Attested.Header.Hash(), vhead.Slot, vhead.Hash()) } } for i := len(expHeads); i < len(ht.validated); i++ { - vhead := ht.validated[i].Header + vhead := ht.validated[i].Attested.Header t.Errorf("Unexpected validated head in test case #%d index #%d (expected none, got {slot %d blockRoot %x})", tci, i, vhead.Slot, vhead.Hash()) } ht.validated = nil diff --git a/beacon/light/sync/types.go b/beacon/light/sync/types.go index 8aa4c95f46..97a3fb2111 100644 --- a/beacon/light/sync/types.go +++ b/beacon/light/sync/types.go @@ -23,9 +23,9 @@ import ( ) var ( - EvNewHead = &request.EventType{Name: "newHead"} // data: types.HeadInfo - EvNewSignedHead = &request.EventType{Name: "newSignedHead"} // data: types.SignedHeader - EvNewFinalityUpdate = &request.EventType{Name: "newFinalityUpdate"} // data: types.FinalityUpdate + EvNewHead = &request.EventType{Name: "newHead"} // data: types.HeadInfo + EvNewOptimisticUpdate = &request.EventType{Name: "newOptimisticUpdate"} // data: types.OptimisticUpdate + EvNewFinalityUpdate = &request.EventType{Name: "newFinalityUpdate"} // data: types.FinalityUpdate ) type ( @@ -43,4 +43,5 @@ type ( } ReqCheckpointData common.Hash ReqBeaconBlock common.Hash + ReqFinality struct{} ) diff --git a/beacon/light/sync/update_sync.go b/beacon/light/sync/update_sync.go index 71801b1b60..9549ee5992 100644 --- a/beacon/light/sync/update_sync.go +++ b/beacon/light/sync/update_sync.go @@ -84,6 +84,7 @@ func (s *CheckpointInit) Process(requester request.Requester, events []request.E if s.initialized { return } + for _, event := range events { switch event.Type { case request.EvResponse, request.EvFail, request.EvTimeout: @@ -132,10 +133,12 @@ func (s *CheckpointInit) Process(requester request.Requester, events []request.E newState.state = ssPrintStatus s.serverState[sid.Server] = newState } + case request.EvUnregistered: delete(s.serverState, event.Server) } } + // start a request if possible for _, server := range requester.CanSendTo() { switch s.serverState[server].state { @@ -156,6 +159,7 @@ func (s *CheckpointInit) Process(requester request.Requester, events []request.E s.serverState[server] = newState } } + // print log message if necessary for server, state := range s.serverState { if state.state != ssPrintStatus { @@ -316,9 +320,9 @@ func (s *ForwardUpdateSync) Process(requester request.Requester, events []reques if !queued { s.unlockRange(sid, req) } - case EvNewSignedHead: - signedHead := event.Data.(types.SignedHeader) - s.nextSyncPeriod[event.Server] = types.SyncPeriod(signedHead.SignatureSlot + 256) + case EvNewOptimisticUpdate: + update := event.Data.(types.OptimisticUpdate) + s.nextSyncPeriod[event.Server] = types.SyncPeriod(update.SignatureSlot + 256) case request.EvUnregistered: delete(s.nextSyncPeriod, event.Server) } diff --git a/beacon/light/sync/update_sync_test.go b/beacon/light/sync/update_sync_test.go index 1c4b3d6d76..8329bf28c9 100644 --- a/beacon/light/sync/update_sync_test.go +++ b/beacon/light/sync/update_sync_test.go @@ -68,9 +68,9 @@ func TestUpdateSyncParallel(t *testing.T) { ts := NewTestScheduler(t, updateSync) // add 2 servers, head at period 100; allow 3-3 parallel requests for each ts.AddServer(testServer1, 3) - ts.ServerEvent(EvNewSignedHead, testServer1, types.SignedHeader{SignatureSlot: 0x2000*100 + 0x1000}) + ts.ServerEvent(EvNewOptimisticUpdate, testServer1, types.OptimisticUpdate{SignatureSlot: 0x2000*100 + 0x1000}) ts.AddServer(testServer2, 3) - ts.ServerEvent(EvNewSignedHead, testServer2, types.SignedHeader{SignatureSlot: 0x2000*100 + 0x1000}) + ts.ServerEvent(EvNewOptimisticUpdate, testServer2, types.OptimisticUpdate{SignatureSlot: 0x2000*100 + 0x1000}) // expect 6 requests to be sent ts.Run(1, @@ -150,11 +150,11 @@ func TestUpdateSyncDifferentHeads(t *testing.T) { ts := NewTestScheduler(t, updateSync) // add 3 servers with different announced head periods ts.AddServer(testServer1, 1) - ts.ServerEvent(EvNewSignedHead, testServer1, types.SignedHeader{SignatureSlot: 0x2000*15 + 0x1000}) + ts.ServerEvent(EvNewOptimisticUpdate, testServer1, types.OptimisticUpdate{SignatureSlot: 0x2000*15 + 0x1000}) ts.AddServer(testServer2, 1) - ts.ServerEvent(EvNewSignedHead, testServer2, types.SignedHeader{SignatureSlot: 0x2000*16 + 0x1000}) + ts.ServerEvent(EvNewOptimisticUpdate, testServer2, types.OptimisticUpdate{SignatureSlot: 0x2000*16 + 0x1000}) ts.AddServer(testServer3, 1) - ts.ServerEvent(EvNewSignedHead, testServer3, types.SignedHeader{SignatureSlot: 0x2000*17 + 0x1000}) + ts.ServerEvent(EvNewOptimisticUpdate, testServer3, types.OptimisticUpdate{SignatureSlot: 0x2000*17 + 0x1000}) // expect request to the best announced head ts.Run(1, testServer3, ReqUpdates{FirstPeriod: 10, Count: 7}) @@ -190,7 +190,7 @@ func TestUpdateSyncDifferentHeads(t *testing.T) { // a new server is registered with announced head period 17 ts.AddServer(testServer4, 1) - ts.ServerEvent(EvNewSignedHead, testServer4, types.SignedHeader{SignatureSlot: 0x2000*17 + 0x1000}) + ts.ServerEvent(EvNewOptimisticUpdate, testServer4, types.OptimisticUpdate{SignatureSlot: 0x2000*17 + 0x1000}) // expect request to sync one more period ts.Run(7, testServer4, ReqUpdates{FirstPeriod: 16, Count: 1}) diff --git a/beacon/types/exec_payload.go b/beacon/types/exec_payload.go index 604de288d2..718f98f529 100644 --- a/beacon/types/exec_payload.go +++ b/beacon/types/exec_payload.go @@ -66,9 +66,8 @@ func convertPayload[T payloadType](payload T, parentRoot *zrntcommon.Root) (*typ block := types.NewBlockWithHeader(&header) block = block.WithBody(transactions, nil) block = block.WithWithdrawals(withdrawals) - hash := block.Hash() - if hash != expectedHash { - return block, fmt.Errorf("Sanity check failed, payload hash does not match (expected %x, got %x)", expectedHash, hash) + if hash := block.Hash(); hash != expectedHash { + return nil, fmt.Errorf("Sanity check failed, payload hash does not match (expected %x, got %x)", expectedHash, hash) } return block, nil } diff --git a/beacon/types/light_sync.go b/beacon/types/light_sync.go index 62becdb21c..3e9b13d0e2 100644 --- a/beacon/types/light_sync.go +++ b/beacon/types/light_sync.go @@ -23,7 +23,7 @@ import ( "github.com/ethereum/go-ethereum/beacon/merkle" "github.com/ethereum/go-ethereum/beacon/params" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/core/types" + ctypes "github.com/ethereum/go-ethereum/core/types" ) // HeadInfo represents an unvalidated new head announcement. @@ -142,17 +142,57 @@ func (u UpdateScore) BetterThan(w UpdateScore) bool { return u.SignerCount > w.SignerCount } +// HeaderWithExecProof contains a beacon header and proves the belonging execution +// payload header with a Merkle proof. type HeaderWithExecProof struct { Header PayloadHeader *ExecutionHeader PayloadBranch merkle.Values } +// Validate verifies the Merkle proof of the execution payload header. func (h *HeaderWithExecProof) Validate() error { - payloadRoot := h.PayloadHeader.PayloadRoot() - return merkle.VerifyProof(h.BodyRoot, params.BodyIndexExecPayload, h.PayloadBranch, payloadRoot) + return merkle.VerifyProof(h.BodyRoot, params.BodyIndexExecPayload, h.PayloadBranch, h.PayloadHeader.PayloadRoot()) } +// OptimisticUpdate proves sync committee commitment on the attested beacon header. +// It also proves the belonging execution payload header with a Merkle proof. +// +// See data structure definition here: +// https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md#lightclientoptimisticupdate +type OptimisticUpdate struct { + Attested HeaderWithExecProof + // Sync committee BLS signature aggregate + Signature SyncAggregate + // Slot in which the signature has been created (newer than Header.Slot, + // determines the signing sync committee) + SignatureSlot uint64 +} + +// SignedHeader returns the signed attested header of the update. +func (u *OptimisticUpdate) SignedHeader() SignedHeader { + return SignedHeader{ + Header: u.Attested.Header, + Signature: u.Signature, + SignatureSlot: u.SignatureSlot, + } +} + +// Validate verifies the Merkle proof proving the execution payload header. +// Note that the sync committee signature of the attested header should be +// verified separately by a synced committee chain. +func (u *OptimisticUpdate) Validate() error { + return u.Attested.Validate() +} + +// FinalityUpdate proves a finalized beacon header by a sync committee commitment +// on an attested beacon header, referring to the latest finalized header with a +// Merkle proof. +// It also proves the execution payload header belonging to both the attested and +// the finalized beacon header with Merkle proofs. +// +// See data structure definition here: +// https://github.com/ethereum/consensus-specs/blob/dev/specs/altair/light-client/sync-protocol.md#lightclientfinalityupdate type FinalityUpdate struct { Attested, Finalized HeaderWithExecProof FinalityBranch merkle.Values @@ -163,6 +203,7 @@ type FinalityUpdate struct { SignatureSlot uint64 } +// SignedHeader returns the signed attested header of the update. func (u *FinalityUpdate) SignedHeader() SignedHeader { return SignedHeader{ Header: u.Attested.Header, @@ -171,6 +212,10 @@ func (u *FinalityUpdate) SignedHeader() SignedHeader { } } +// Validate verifies the Merkle proofs proving the finalized beacon header and +// the execution payload headers belonging to the attested and finalized headers. +// Note that the sync committee signature of the attested header should be +// verified separately by a synced committee chain. func (u *FinalityUpdate) Validate() error { if err := u.Attested.Validate(); err != nil { return err @@ -186,6 +231,6 @@ func (u *FinalityUpdate) Validate() error { // finalized execution block. type ChainHeadEvent struct { BeaconHead Header - Block *types.Block + Block *ctypes.Block Finalized common.Hash } From 94579932b18931115f28aa7f87f02450bda084c9 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Tue, 23 Apr 2024 07:10:24 -0600 Subject: [PATCH 06/15] core/vm: fix Prague contracts (#29612) core/vm: fix prague contracts --- core/vm/contracts.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/vm/contracts.go b/core/vm/contracts.go index 3d4819a74b..5f7de8007b 100644 --- a/core/vm/contracts.go +++ b/core/vm/contracts.go @@ -114,6 +114,16 @@ var PrecompiledContractsCancun = map[common.Address]PrecompiledContract{ // PrecompiledContractsPrague contains the set of pre-compiled Ethereum // contracts used in the Prague release. var PrecompiledContractsPrague = map[common.Address]PrecompiledContract{ + common.BytesToAddress([]byte{0x01}): &ecrecover{}, + common.BytesToAddress([]byte{0x02}): &sha256hash{}, + common.BytesToAddress([]byte{0x03}): &ripemd160hash{}, + common.BytesToAddress([]byte{0x04}): &dataCopy{}, + common.BytesToAddress([]byte{0x05}): &bigModExp{eip2565: true}, + common.BytesToAddress([]byte{0x06}): &bn256AddIstanbul{}, + common.BytesToAddress([]byte{0x07}): &bn256ScalarMulIstanbul{}, + common.BytesToAddress([]byte{0x08}): &bn256PairingIstanbul{}, + common.BytesToAddress([]byte{0x09}): &blake2F{}, + common.BytesToAddress([]byte{0x0a}): &kzgPointEvaluation{}, common.BytesToAddress([]byte{0x0b}): &bls12381G1Add{}, common.BytesToAddress([]byte{0x0c}): &bls12381G1Mul{}, common.BytesToAddress([]byte{0x0d}): &bls12381G1MultiExp{}, From 882d1e22f66521d62ecdfe4566fb419765d744cf Mon Sep 17 00:00:00 2001 From: Chris Ziogas Date: Wed, 24 Apr 2024 08:53:16 +0300 Subject: [PATCH 07/15] cmd/geth, cmd/utils: rename config and flag to` VMTraceJsonConfig` (#29573) renames the yaml config field VMTraceConfig to VMTraceJsonConfig, in order to be consistent with the renaming of the CLI flag. --- cmd/geth/chaincmd.go | 2 +- cmd/geth/main.go | 2 +- cmd/utils/flags.go | 12 ++++++------ eth/backend.go | 4 ++-- eth/ethconfig/config.go | 4 ++-- eth/ethconfig/gen_config.go | 10 +++++----- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cmd/geth/chaincmd.go b/cmd/geth/chaincmd.go index dc45661eae..d787f340a3 100644 --- a/cmd/geth/chaincmd.go +++ b/cmd/geth/chaincmd.go @@ -100,7 +100,7 @@ if one is set. Otherwise it prints the genesis from the datadir.`, utils.MetricsInfluxDBOrganizationFlag, utils.TxLookupLimitFlag, utils.VMTraceFlag, - utils.VMTraceConfigFlag, + utils.VMTraceJsonConfigFlag, utils.TransactionHistoryFlag, utils.StateHistoryFlag, }, utils.DatabaseFlags), diff --git a/cmd/geth/main.go b/cmd/geth/main.go index 37b99a2621..b7885608bc 100644 --- a/cmd/geth/main.go +++ b/cmd/geth/main.go @@ -137,7 +137,7 @@ var ( utils.DeveloperPeriodFlag, utils.VMEnableDebugFlag, utils.VMTraceFlag, - utils.VMTraceConfigFlag, + utils.VMTraceJsonConfigFlag, utils.NetworkIdFlag, utils.EthStatsURLFlag, utils.NoCompactionFlag, diff --git a/cmd/utils/flags.go b/cmd/utils/flags.go index 8ef9d9e7e8..e1c33678be 100644 --- a/cmd/utils/flags.go +++ b/cmd/utils/flags.go @@ -544,7 +544,7 @@ var ( Usage: "Name of tracer which should record internal VM operations (costly)", Category: flags.VMCategory, } - VMTraceConfigFlag = &cli.StringFlag{ + VMTraceJsonConfigFlag = &cli.StringFlag{ Name: "vmtrace.jsonconfig", Usage: "Tracer configuration (JSON)", Category: flags.VMCategory, @@ -1903,12 +1903,12 @@ func SetEthConfig(ctx *cli.Context, stack *node.Node, cfg *ethconfig.Config) { if ctx.IsSet(VMTraceFlag.Name) { if name := ctx.String(VMTraceFlag.Name); name != "" { var config string - if ctx.IsSet(VMTraceConfigFlag.Name) { - config = ctx.String(VMTraceConfigFlag.Name) + if ctx.IsSet(VMTraceJsonConfigFlag.Name) { + config = ctx.String(VMTraceJsonConfigFlag.Name) } cfg.VMTrace = name - cfg.VMTraceConfig = config + cfg.VMTraceJsonConfig = config } } } @@ -2192,8 +2192,8 @@ func MakeChain(ctx *cli.Context, stack *node.Node, readonly bool) (*core.BlockCh if ctx.IsSet(VMTraceFlag.Name) { if name := ctx.String(VMTraceFlag.Name); name != "" { var config json.RawMessage - if ctx.IsSet(VMTraceConfigFlag.Name) { - config = json.RawMessage(ctx.String(VMTraceConfigFlag.Name)) + if ctx.IsSet(VMTraceJsonConfigFlag.Name) { + config = json.RawMessage(ctx.String(VMTraceJsonConfigFlag.Name)) } t, err := tracers.LiveDirectory.New(name, config) if err != nil { diff --git a/eth/backend.go b/eth/backend.go index 04ee82efee..e616b5f2f1 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -203,8 +203,8 @@ func New(stack *node.Node, config *ethconfig.Config) (*Ethereum, error) { ) if config.VMTrace != "" { var traceConfig json.RawMessage - if config.VMTraceConfig != "" { - traceConfig = json.RawMessage(config.VMTraceConfig) + if config.VMTraceJsonConfig != "" { + traceConfig = json.RawMessage(config.VMTraceJsonConfig) } t, err := tracers.LiveDirectory.New(config.VMTrace, traceConfig) if err != nil { diff --git a/eth/ethconfig/config.go b/eth/ethconfig/config.go index 91f2c8d330..f36f212d9c 100644 --- a/eth/ethconfig/config.go +++ b/eth/ethconfig/config.go @@ -142,8 +142,8 @@ type Config struct { EnablePreimageRecording bool // Enables VM tracing - VMTrace string - VMTraceConfig string + VMTrace string + VMTraceJsonConfig string // Miscellaneous options DocRoot string `toml:"-"` diff --git a/eth/ethconfig/gen_config.go b/eth/ethconfig/gen_config.go index cda62bc9dd..b8b9eee294 100644 --- a/eth/ethconfig/gen_config.go +++ b/eth/ethconfig/gen_config.go @@ -51,7 +51,7 @@ func (c Config) MarshalTOML() (interface{}, error) { GPO gasprice.Config EnablePreimageRecording bool VMTrace string - VMTraceConfig string + VMTraceJsonConfig string DocRoot string `toml:"-"` RPCGasCap uint64 RPCEVMTimeout time.Duration @@ -94,7 +94,7 @@ func (c Config) MarshalTOML() (interface{}, error) { enc.GPO = c.GPO enc.EnablePreimageRecording = c.EnablePreimageRecording enc.VMTrace = c.VMTrace - enc.VMTraceConfig = c.VMTraceConfig + enc.VMTraceJsonConfig = c.VMTraceJsonConfig enc.DocRoot = c.DocRoot enc.RPCGasCap = c.RPCGasCap enc.RPCEVMTimeout = c.RPCEVMTimeout @@ -141,7 +141,7 @@ func (c *Config) UnmarshalTOML(unmarshal func(interface{}) error) error { GPO *gasprice.Config EnablePreimageRecording *bool VMTrace *string - VMTraceConfig *string + VMTraceJsonConfig *string DocRoot *string `toml:"-"` RPCGasCap *uint64 RPCEVMTimeout *time.Duration @@ -255,8 +255,8 @@ func (c *Config) UnmarshalTOML(unmarshal func(interface{}) error) error { if dec.VMTrace != nil { c.VMTrace = *dec.VMTrace } - if dec.VMTraceConfig != nil { - c.VMTraceConfig = *dec.VMTraceConfig + if dec.VMTraceJsonConfig != nil { + c.VMTraceJsonConfig = *dec.VMTraceJsonConfig } if dec.DocRoot != nil { c.DocRoot = *dec.DocRoot From fb08fd334a48e2ad07e0d5205c23368ec9cd2bec Mon Sep 17 00:00:00 2001 From: Chris Ziogas Date: Wed, 24 Apr 2024 08:54:59 +0300 Subject: [PATCH 08/15] core/tracing: Add OnClose Trace Hook (#29629) The OnClose trace hook is being triggered on blockchain Stop, so as tracers can release any resources. --- core/blockchain.go | 4 ++++ core/tracing/hooks.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/core/blockchain.go b/core/blockchain.go index 8c97740752..b45cd92e52 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1153,6 +1153,10 @@ func (bc *BlockChain) Stop() { } } } + // Allow tracers to clean-up and release resources. + if bc.logger != nil && bc.logger.OnClose != nil { + bc.logger.OnClose() + } // Close the trie database, release all the held resources as the last step. if err := bc.triedb.Close(); err != nil { log.Error("Failed to close trie database", "err", err) diff --git a/core/tracing/hooks.go b/core/tracing/hooks.go index 48cb4d2027..9ca6ee39fb 100644 --- a/core/tracing/hooks.go +++ b/core/tracing/hooks.go @@ -107,6 +107,9 @@ type ( // BlockchainInitHook is called when the blockchain is initialized. BlockchainInitHook = func(chainConfig *params.ChainConfig) + // CloseHook is called when the blockchain closes. + CloseHook = func() + // BlockStartHook is called before executing `block`. // `td` is the total difficulty prior to `block`. BlockStartHook = func(event BlockEvent) @@ -153,6 +156,7 @@ type Hooks struct { OnGasChange GasChangeHook // Chain events OnBlockchainInit BlockchainInitHook + OnClose CloseHook OnBlockStart BlockStartHook OnBlockEnd BlockEndHook OnSkippedBlock SkippedBlockHook From ade7515c812e96467dce51da39af346aa27df575 Mon Sep 17 00:00:00 2001 From: Matthieu Vachon Date: Wed, 24 Apr 2024 01:58:05 -0400 Subject: [PATCH 09/15] eth, eth/tracers: process beacon root before transactions (#29402) The beacon root when applied in `state_processor.go` is performed right before executing transaction. That means that contract reliying on this value would query the same value found in the block header. In that spirit, it means that any tracing/operation relying on state data which touches transaction must have updated the beacon root before any transaction processing. --- eth/state_accessor.go | 6 ++++++ eth/tracers/api.go | 22 +++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/eth/state_accessor.go b/eth/state_accessor.go index 770532cbfe..372c76f496 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -233,6 +233,12 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, if err != nil { return nil, vm.BlockContext{}, nil, nil, err } + // Insert parent beacon block root in the state as per EIP-4788. + if beaconRoot := block.BeaconRoot(); beaconRoot != nil { + context := core.NewEVMBlockContext(block.Header(), eth.blockchain, nil) + vmenv := vm.NewEVM(context, vm.TxContext{}, statedb, eth.blockchain.Config(), vm.Config{}) + core.ProcessBeaconBlockRoot(*beaconRoot, vmenv, statedb) + } if txIndex == 0 && len(block.Transactions()) == 0 { return nil, vm.BlockContext{}, statedb, release, nil } diff --git a/eth/tracers/api.go b/eth/tracers/api.go index 7a7c5e48d9..d99531d48f 100644 --- a/eth/tracers/api.go +++ b/eth/tracers/api.go @@ -376,6 +376,13 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed failed = err break } + // Insert block's parent beacon block root in the state + // as per EIP-4788. + if beaconRoot := next.BeaconRoot(); beaconRoot != nil { + context := core.NewEVMBlockContext(next.Header(), api.chainContext(ctx), nil) + vmenv := vm.NewEVM(context, vm.TxContext{}, statedb, api.backend.ChainConfig(), vm.Config{}) + core.ProcessBeaconBlockRoot(*beaconRoot, vmenv, statedb) + } // Clean out any pending release functions of trace state. Note this // step must be done after constructing tracing state, because the // tracing state of block next depends on the parent state and construction @@ -517,7 +524,6 @@ func (api *API) IntermediateRoots(ctx context.Context, hash common.Hash, config return nil, err } defer release() - var ( roots []common.Hash signer = types.MakeSigner(api.backend.ChainConfig(), block.Number(), block.Time()) @@ -525,6 +531,10 @@ func (api *API) IntermediateRoots(ctx context.Context, hash common.Hash, config vmctx = core.NewEVMBlockContext(block.Header(), api.chainContext(ctx), nil) deleteEmptyObjects = chainConfig.IsEIP158(block.Number()) ) + if beaconRoot := block.BeaconRoot(); beaconRoot != nil { + vmenv := vm.NewEVM(vmctx, vm.TxContext{}, statedb, chainConfig, vm.Config{}) + core.ProcessBeaconBlockRoot(*beaconRoot, vmenv, statedb) + } for i, tx := range block.Transactions() { if err := ctx.Err(); err != nil { return nil, err @@ -584,7 +594,6 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac return nil, err } defer release() - // JS tracers have high overhead. In this case run a parallel // process that generates states in one thread and traces txes // in separate worker threads. @@ -601,6 +610,10 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac signer = types.MakeSigner(api.backend.ChainConfig(), block.Number(), block.Time()) results = make([]*txTraceResult, len(txs)) ) + if beaconRoot := block.BeaconRoot(); beaconRoot != nil { + vmenv := vm.NewEVM(blockCtx, vm.TxContext{}, statedb, api.backend.ChainConfig(), vm.Config{}) + core.ProcessBeaconBlockRoot(*beaconRoot, vmenv, statedb) + } for i, tx := range txs { // Generate the next state snapshot fast without tracing msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee()) @@ -727,7 +740,6 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block return nil, err } defer release() - // Retrieve the tracing configurations, or use default values var ( logConfig logger.Config @@ -756,6 +768,10 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block // Note: This copies the config, to not screw up the main config chainConfig, canon = overrideConfig(chainConfig, config.Overrides) } + if beaconRoot := block.BeaconRoot(); beaconRoot != nil { + vmenv := vm.NewEVM(vmctx, vm.TxContext{}, statedb, chainConfig, vm.Config{}) + core.ProcessBeaconBlockRoot(*beaconRoot, vmenv, statedb) + } for i, tx := range block.Transactions() { // Prepare the transaction for un-traced execution var ( From 5f3c58f1de9ef91dd22b632e6bcb559081d56e1d Mon Sep 17 00:00:00 2001 From: jwasinger Date: Wed, 24 Apr 2024 00:07:39 -0700 Subject: [PATCH 10/15] eth/downloader: fix case where skeleton reorgs below the filled block (#29358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change adds a testcase and fixes a corner-case in the skeleton sync. With this change, when doing the skeleton cleanup, we check if the filled header is acually within the range of what we were meant to backfill. If not, it means the backfill was a noop (possibly because we started and stopped it so quickly that it didn't have time to do any meaningful work). In that case, just don't clean up anything. --------- Co-authored-by: Péter Szilágyi --- eth/downloader/downloader_test.go | 81 +++++++++++++++++++++++++++++++ eth/downloader/skeleton.go | 10 ++++ 2 files changed, 91 insertions(+) diff --git a/eth/downloader/downloader_test.go b/eth/downloader/downloader_test.go index 0fdc7ead9c..c810518d56 100644 --- a/eth/downloader/downloader_test.go +++ b/eth/downloader/downloader_test.go @@ -1311,3 +1311,84 @@ func testBeaconSync(t *testing.T, protocol uint, mode SyncMode) { }) } } + +// Tests that synchronisation progress (origin block number and highest block +// number) is tracked and updated correctly in case of manual head reversion +func TestBeaconForkedSyncProgress68Full(t *testing.T) { + testBeaconForkedSyncProgress(t, eth.ETH68, FullSync) +} +func TestBeaconForkedSyncProgress68Snap(t *testing.T) { + testBeaconForkedSyncProgress(t, eth.ETH68, SnapSync) +} +func TestBeaconForkedSyncProgress68Light(t *testing.T) { + testBeaconForkedSyncProgress(t, eth.ETH68, LightSync) +} + +func testBeaconForkedSyncProgress(t *testing.T, protocol uint, mode SyncMode) { + success := make(chan struct{}) + tester := newTesterWithNotification(t, func() { + success <- struct{}{} + }) + defer tester.terminate() + + chainA := testChainForkLightA.shorten(len(testChainBase.blocks) + MaxHeaderFetch) + chainB := testChainForkLightB.shorten(len(testChainBase.blocks) + MaxHeaderFetch) + + // Set a sync init hook to catch progress changes + starting := make(chan struct{}) + progress := make(chan struct{}) + + tester.downloader.syncInitHook = func(origin, latest uint64) { + starting <- struct{}{} + <-progress + } + checkProgress(t, tester.downloader, "pristine", ethereum.SyncProgress{}) + + // Synchronise with one of the forks and check progress + tester.newPeer("fork A", protocol, chainA.blocks[1:]) + pending := new(sync.WaitGroup) + pending.Add(1) + go func() { + defer pending.Done() + if err := tester.downloader.BeaconSync(mode, chainA.blocks[len(chainA.blocks)-1].Header(), nil); err != nil { + panic(fmt.Sprintf("failed to beacon sync: %v", err)) + } + }() + + <-starting + progress <- struct{}{} + select { + case <-success: + checkProgress(t, tester.downloader, "initial", ethereum.SyncProgress{ + HighestBlock: uint64(len(chainA.blocks) - 1), + CurrentBlock: uint64(len(chainA.blocks) - 1), + }) + case <-time.NewTimer(time.Second * 3).C: + t.Fatalf("Failed to sync chain in three seconds") + } + + // Set the head to a second fork + tester.newPeer("fork B", protocol, chainB.blocks[1:]) + pending.Add(1) + go func() { + defer pending.Done() + if err := tester.downloader.BeaconSync(mode, chainB.blocks[len(chainB.blocks)-1].Header(), nil); err != nil { + panic(fmt.Sprintf("failed to beacon sync: %v", err)) + } + }() + + <-starting + progress <- struct{}{} + + // reorg below available state causes the state sync to rewind to genesis + select { + case <-success: + checkProgress(t, tester.downloader, "initial", ethereum.SyncProgress{ + HighestBlock: uint64(len(chainB.blocks) - 1), + CurrentBlock: uint64(len(chainB.blocks) - 1), + StartingBlock: 0, + }) + case <-time.NewTimer(time.Second * 3).C: + t.Fatalf("Failed to sync chain in three seconds") + } +} diff --git a/eth/downloader/skeleton.go b/eth/downloader/skeleton.go index 873ee950b6..04421a2bf5 100644 --- a/eth/downloader/skeleton.go +++ b/eth/downloader/skeleton.go @@ -1132,6 +1132,16 @@ func (s *skeleton) cleanStales(filled *types.Header) error { if number+1 == s.progress.Subchains[0].Tail { return nil } + // If the latest fill was on a different subchain, it means the backfiller + // was interrupted before it got to do any meaningful work, no cleanup + header := rawdb.ReadSkeletonHeader(s.db, filled.Number.Uint64()) + if header == nil { + log.Debug("Filled header outside of skeleton range", "number", number, "head", s.progress.Subchains[0].Head, "tail", s.progress.Subchains[0].Tail) + return nil + } else if header.Hash() != filled.Hash() { + log.Debug("Filled header on different sidechain", "number", number, "filled", filled.Hash(), "skeleton", header.Hash()) + return nil + } var ( start uint64 end uint64 From 87246f3cbaf10f83f56bc4d45f0f3e36e83e71e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 24 Apr 2024 11:02:49 +0300 Subject: [PATCH 11/15] params: release Geth v1.14.0 --- params/version.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/params/version.go b/params/version.go index a49385da7d..b3978be046 100644 --- a/params/version.go +++ b/params/version.go @@ -21,10 +21,10 @@ import ( ) const ( - VersionMajor = 1 // Major version component of the current release - VersionMinor = 14 // Minor version component of the current release - VersionPatch = 0 // Patch version component of the current release - VersionMeta = "unstable" // Version metadata to append to the version string + VersionMajor = 1 // Major version component of the current release + VersionMinor = 14 // Minor version component of the current release + VersionPatch = 0 // Patch version component of the current release + VersionMeta = "stable" // Version metadata to append to the version string ) // Version holds the textual version string. From 938734be3ce1e254a0a72e56f8bdfb41b58e8953 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 24 Apr 2024 11:05:10 +0300 Subject: [PATCH 12/15] params: begin 1.14.1 release cycle --- params/version.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/params/version.go b/params/version.go index b3978be046..319f21b2a8 100644 --- a/params/version.go +++ b/params/version.go @@ -21,10 +21,10 @@ import ( ) const ( - VersionMajor = 1 // Major version component of the current release - VersionMinor = 14 // Minor version component of the current release - VersionPatch = 0 // Patch version component of the current release - VersionMeta = "stable" // Version metadata to append to the version string + VersionMajor = 1 // Major version component of the current release + VersionMinor = 14 // Minor version component of the current release + VersionPatch = 1 // Patch version component of the current release + VersionMeta = "unstable" // Version metadata to append to the version string ) // Version holds the textual version string. From 0d4c38865e9cda492e71221c4c429d9b1bec8ac5 Mon Sep 17 00:00:00 2001 From: Martin HS Date: Wed, 24 Apr 2024 11:59:06 +0200 Subject: [PATCH 13/15] core/state: remove account reset operation v2 (#29520) * core/state, tests: remove account reset operation * core/state, core/vm: implement createcontract journal event * core/state: make createcontract not emit dirtied account, unskip tests * core/state: add createcontract to journal fuzzing * core/state: fix journal * core/state: address comments * core/state: remove useless code --------- Co-authored-by: Gary Rong --- core/state/journal.go | 151 ++++++++++++++---- core/state/state_object.go | 54 +++---- core/state/state_test.go | 102 +----------- core/state/statedb.go | 270 ++++++++++++++++---------------- core/state/statedb_fuzz_test.go | 4 +- core/state/statedb_test.go | 173 ++++++++++++++------ core/vm/evm.go | 21 ++- core/vm/interface.go | 1 + tests/block_test.go | 8 - 9 files changed, 426 insertions(+), 358 deletions(-) diff --git a/core/state/journal.go b/core/state/journal.go index 6cdc1fc868..cfd3782eb0 100644 --- a/core/state/journal.go +++ b/core/state/journal.go @@ -17,6 +17,8 @@ package state import ( + "maps" + "github.com/ethereum/go-ethereum/common" "github.com/holiman/uint256" ) @@ -29,6 +31,9 @@ type journalEntry interface { // dirtied returns the Ethereum address modified by this journal entry. dirtied() *common.Address + + // copy returns a deep-copied journal entry. + copy() journalEntry } // journal contains the list of state modifications applied since the last state @@ -83,22 +88,31 @@ func (j *journal) length() int { return len(j.entries) } +// copy returns a deep-copied journal. +func (j *journal) copy() *journal { + entries := make([]journalEntry, 0, j.length()) + for i := 0; i < j.length(); i++ { + entries = append(entries, j.entries[i].copy()) + } + return &journal{ + entries: entries, + dirties: maps.Clone(j.dirties), + } +} + type ( // Changes to the account trie. createObjectChange struct { account *common.Address } - resetObjectChange struct { - account *common.Address - prev *stateObject - prevdestruct bool - prevAccount []byte - prevStorage map[common.Hash][]byte - prevAccountOriginExist bool - prevAccountOrigin []byte - prevStorageOrigin map[common.Hash][]byte + // createContractChange represents an account becoming a contract-account. + // This event happens prior to executing initcode. The journal-event simply + // manages the created-flag, in order to allow same-tx destruction. + createContractChange struct { + account common.Address } + selfDestructChange struct { account *common.Address prev bool // whether account had already self-destructed @@ -136,6 +150,7 @@ type ( touchChange struct { account *common.Address } + // Changes to the access list accessListAddAccountChange struct { address *common.Address @@ -145,6 +160,7 @@ type ( slot *common.Hash } + // Changes to transient storage transientStorageChange struct { account *common.Address key, prevalue common.Hash @@ -153,34 +169,30 @@ type ( func (ch createObjectChange) revert(s *StateDB) { delete(s.stateObjects, *ch.account) - delete(s.stateObjectsDirty, *ch.account) } func (ch createObjectChange) dirtied() *common.Address { return ch.account } -func (ch resetObjectChange) revert(s *StateDB) { - s.setStateObject(ch.prev) - if !ch.prevdestruct { - delete(s.stateObjectsDestruct, ch.prev.address) - } - if ch.prevAccount != nil { - s.accounts[ch.prev.addrHash] = ch.prevAccount - } - if ch.prevStorage != nil { - s.storages[ch.prev.addrHash] = ch.prevStorage - } - if ch.prevAccountOriginExist { - s.accountsOrigin[ch.prev.address] = ch.prevAccountOrigin - } - if ch.prevStorageOrigin != nil { - s.storagesOrigin[ch.prev.address] = ch.prevStorageOrigin +func (ch createObjectChange) copy() journalEntry { + return createObjectChange{ + account: ch.account, } } -func (ch resetObjectChange) dirtied() *common.Address { - return ch.account +func (ch createContractChange) revert(s *StateDB) { + s.getStateObject(ch.account).newContract = false +} + +func (ch createContractChange) dirtied() *common.Address { + return nil +} + +func (ch createContractChange) copy() journalEntry { + return createContractChange{ + account: ch.account, + } } func (ch selfDestructChange) revert(s *StateDB) { @@ -195,6 +207,14 @@ func (ch selfDestructChange) dirtied() *common.Address { return ch.account } +func (ch selfDestructChange) copy() journalEntry { + return selfDestructChange{ + account: ch.account, + prev: ch.prev, + prevbalance: new(uint256.Int).Set(ch.prevbalance), + } +} + var ripemd = common.HexToAddress("0000000000000000000000000000000000000003") func (ch touchChange) revert(s *StateDB) { @@ -204,6 +224,12 @@ func (ch touchChange) dirtied() *common.Address { return ch.account } +func (ch touchChange) copy() journalEntry { + return touchChange{ + account: ch.account, + } +} + func (ch balanceChange) revert(s *StateDB) { s.getStateObject(*ch.account).setBalance(ch.prev) } @@ -212,6 +238,13 @@ func (ch balanceChange) dirtied() *common.Address { return ch.account } +func (ch balanceChange) copy() journalEntry { + return balanceChange{ + account: ch.account, + prev: new(uint256.Int).Set(ch.prev), + } +} + func (ch nonceChange) revert(s *StateDB) { s.getStateObject(*ch.account).setNonce(ch.prev) } @@ -220,6 +253,13 @@ func (ch nonceChange) dirtied() *common.Address { return ch.account } +func (ch nonceChange) copy() journalEntry { + return nonceChange{ + account: ch.account, + prev: ch.prev, + } +} + func (ch codeChange) revert(s *StateDB) { s.getStateObject(*ch.account).setCode(common.BytesToHash(ch.prevhash), ch.prevcode) } @@ -228,6 +268,14 @@ func (ch codeChange) dirtied() *common.Address { return ch.account } +func (ch codeChange) copy() journalEntry { + return codeChange{ + account: ch.account, + prevhash: common.CopyBytes(ch.prevhash), + prevcode: common.CopyBytes(ch.prevcode), + } +} + func (ch storageChange) revert(s *StateDB) { s.getStateObject(*ch.account).setState(ch.key, ch.prevalue) } @@ -236,6 +284,14 @@ func (ch storageChange) dirtied() *common.Address { return ch.account } +func (ch storageChange) copy() journalEntry { + return storageChange{ + account: ch.account, + key: ch.key, + prevalue: ch.prevalue, + } +} + func (ch transientStorageChange) revert(s *StateDB) { s.setTransientState(*ch.account, ch.key, ch.prevalue) } @@ -244,6 +300,14 @@ func (ch transientStorageChange) dirtied() *common.Address { return nil } +func (ch transientStorageChange) copy() journalEntry { + return transientStorageChange{ + account: ch.account, + key: ch.key, + prevalue: ch.prevalue, + } +} + func (ch refundChange) revert(s *StateDB) { s.refund = ch.prev } @@ -252,6 +316,12 @@ func (ch refundChange) dirtied() *common.Address { return nil } +func (ch refundChange) copy() journalEntry { + return refundChange{ + prev: ch.prev, + } +} + func (ch addLogChange) revert(s *StateDB) { logs := s.logs[ch.txhash] if len(logs) == 1 { @@ -266,6 +336,12 @@ func (ch addLogChange) dirtied() *common.Address { return nil } +func (ch addLogChange) copy() journalEntry { + return addLogChange{ + txhash: ch.txhash, + } +} + func (ch addPreimageChange) revert(s *StateDB) { delete(s.preimages, ch.hash) } @@ -274,6 +350,12 @@ func (ch addPreimageChange) dirtied() *common.Address { return nil } +func (ch addPreimageChange) copy() journalEntry { + return addPreimageChange{ + hash: ch.hash, + } +} + func (ch accessListAddAccountChange) revert(s *StateDB) { /* One important invariant here, is that whenever a (addr, slot) is added, if the @@ -291,6 +373,12 @@ func (ch accessListAddAccountChange) dirtied() *common.Address { return nil } +func (ch accessListAddAccountChange) copy() journalEntry { + return accessListAddAccountChange{ + address: ch.address, + } +} + func (ch accessListAddSlotChange) revert(s *StateDB) { s.accessList.DeleteSlot(*ch.address, *ch.slot) } @@ -298,3 +386,10 @@ func (ch accessListAddSlotChange) revert(s *StateDB) { func (ch accessListAddSlotChange) dirtied() *common.Address { return nil } + +func (ch accessListAddSlotChange) copy() journalEntry { + return accessListAddSlotChange{ + address: ch.address, + slot: ch.slot, + } +} diff --git a/core/state/state_object.go b/core/state/state_object.go index db3c32f2f2..8978b30542 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -32,21 +32,8 @@ import ( "github.com/holiman/uint256" ) -type Code []byte - -func (c Code) String() string { - return string(c) //strings.Join(Disassemble(c), " ") -} - type Storage map[common.Hash]common.Hash -func (s Storage) String() (str string) { - for key, value := range s { - str += fmt.Sprintf("%X : %X\n", key, value) - } - return -} - func (s Storage) Copy() Storage { return maps.Clone(s) } @@ -65,8 +52,8 @@ type stateObject struct { data types.StateAccount // Account data with all mutations applied in the scope of block // Write caches. - trie Trie // storage trie, which becomes non-nil on first access - code Code // contract bytecode, which gets set when code is loaded + trie Trie // storage trie, which becomes non-nil on first access + code []byte // contract bytecode, which gets set when code is loaded originStorage Storage // Storage cache of original entries to dedup rewrites pendingStorage Storage // Storage entries that need to be flushed to disk, at the end of an entire block @@ -75,17 +62,16 @@ type stateObject struct { // Cache flags. dirtyCode bool // true if the code was updated - // Flag whether the account was marked as self-destructed. The self-destructed account - // is still accessible in the scope of same transaction. + // Flag whether the account was marked as self-destructed. The self-destructed + // account is still accessible in the scope of same transaction. selfDestructed bool - // Flag whether the account was marked as deleted. A self-destructed account - // or an account that is considered as empty will be marked as deleted at - // the end of transaction and no longer accessible anymore. - deleted bool - - // Flag whether the object was created in the current transaction - created bool + // This is an EIP-6780 flag indicating whether the object is eligible for + // self-destruct according to EIP-6780. The flag could be set either when + // the contract is just created within the current transaction, or when the + // object was previously existent and is being deployed as a contract within + // the current transaction. + newContract bool } // empty returns whether the account is considered empty. @@ -95,10 +81,7 @@ func (s *stateObject) empty() bool { // newObject creates a state object. func newObject(db *StateDB, address common.Address, acct *types.StateAccount) *stateObject { - var ( - origin = acct - created = acct == nil // true if the account was not existent - ) + origin := acct if acct == nil { acct = types.NewEmptyStateAccount() } @@ -111,7 +94,6 @@ func newObject(db *StateDB, address common.Address, acct *types.StateAccount) *s originStorage: make(Storage), pendingStorage: make(Storage), dirtyStorage: make(Storage), - created: created, } } @@ -264,6 +246,10 @@ func (s *stateObject) finalise(prefetch bool) { if len(s.dirtyStorage) > 0 { s.dirtyStorage = make(Storage) } + // Revoke the flag at the end of the transaction. It finalizes the status + // of the newly-created object as it's no longer eligible for self-destruct + // by EIP-6780. For non-newly-created objects, it's a no-op. + s.newContract = false } // updateTrie is responsible for persisting cached storage changes into the @@ -463,12 +449,12 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject { obj.trie = db.db.CopyTrie(s.trie) } obj.code = s.code - obj.dirtyStorage = s.dirtyStorage.Copy() obj.originStorage = s.originStorage.Copy() obj.pendingStorage = s.pendingStorage.Copy() - obj.selfDestructed = s.selfDestructed + obj.dirtyStorage = s.dirtyStorage.Copy() obj.dirtyCode = s.dirtyCode - obj.deleted = s.deleted + obj.selfDestructed = s.selfDestructed + obj.newContract = s.newContract return obj } @@ -483,7 +469,7 @@ func (s *stateObject) Address() common.Address { // Code returns the contract code associated with this object, if any. func (s *stateObject) Code() []byte { - if s.code != nil { + if len(s.code) != 0 { return s.code } if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) { @@ -501,7 +487,7 @@ func (s *stateObject) Code() []byte { // or zero if none. This method is an almost mirror of Code, but uses a cache // inside the database to avoid loading codes seen recently. func (s *stateObject) CodeSize() int { - if s.code != nil { + if len(s.code) != 0 { return len(s.code) } if bytes.Equal(s.CodeHash(), types.EmptyCodeHash.Bytes()) { diff --git a/core/state/state_test.go b/core/state/state_test.go index c6e6db906e..9200e4abe9 100644 --- a/core/state/state_test.go +++ b/core/state/state_test.go @@ -194,106 +194,20 @@ func TestSnapshotEmpty(t *testing.T) { s.state.RevertToSnapshot(s.state.Snapshot()) } -func TestSnapshot2(t *testing.T) { +func TestCreateObjectRevert(t *testing.T) { state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil) + addr := common.BytesToAddress([]byte("so0")) + snap := state.Snapshot() - stateobjaddr0 := common.BytesToAddress([]byte("so0")) - stateobjaddr1 := common.BytesToAddress([]byte("so1")) - var storageaddr common.Hash - - data0 := common.BytesToHash([]byte{17}) - data1 := common.BytesToHash([]byte{18}) - - state.SetState(stateobjaddr0, storageaddr, data0) - state.SetState(stateobjaddr1, storageaddr, data1) - - // db, trie are already non-empty values - so0 := state.getStateObject(stateobjaddr0) + state.CreateAccount(addr) + so0 := state.getStateObject(addr) so0.SetBalance(uint256.NewInt(42), tracing.BalanceChangeUnspecified) so0.SetNonce(43) so0.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e'}), []byte{'c', 'a', 'f', 'e'}) - so0.selfDestructed = false - so0.deleted = false state.setStateObject(so0) - root, _ := state.Commit(0, false) - state, _ = New(root, state.db, state.snaps) - - // and one with deleted == true - so1 := state.getStateObject(stateobjaddr1) - so1.SetBalance(uint256.NewInt(52), tracing.BalanceChangeUnspecified) - so1.SetNonce(53) - so1.SetCode(crypto.Keccak256Hash([]byte{'c', 'a', 'f', 'e', '2'}), []byte{'c', 'a', 'f', 'e', '2'}) - so1.selfDestructed = true - so1.deleted = true - state.setStateObject(so1) - - so1 = state.getStateObject(stateobjaddr1) - if so1 != nil { - t.Fatalf("deleted object not nil when getting") - } - - snapshot := state.Snapshot() - state.RevertToSnapshot(snapshot) - - so0Restored := state.getStateObject(stateobjaddr0) - // Update lazily-loaded values before comparing. - so0Restored.GetState(storageaddr) - so0Restored.Code() - // non-deleted is equal (restored) - compareStateObjects(so0Restored, so0, t) - - // deleted should be nil, both before and after restore of state copy - so1Restored := state.getStateObject(stateobjaddr1) - if so1Restored != nil { - t.Fatalf("deleted object not nil after restoring snapshot: %+v", so1Restored) - } -} - -func compareStateObjects(so0, so1 *stateObject, t *testing.T) { - if so0.Address() != so1.Address() { - t.Fatalf("Address mismatch: have %v, want %v", so0.address, so1.address) - } - if so0.Balance().Cmp(so1.Balance()) != 0 { - t.Fatalf("Balance mismatch: have %v, want %v", so0.Balance(), so1.Balance()) - } - if so0.Nonce() != so1.Nonce() { - t.Fatalf("Nonce mismatch: have %v, want %v", so0.Nonce(), so1.Nonce()) - } - if so0.data.Root != so1.data.Root { - t.Errorf("Root mismatch: have %x, want %x", so0.data.Root[:], so1.data.Root[:]) - } - if !bytes.Equal(so0.CodeHash(), so1.CodeHash()) { - t.Fatalf("CodeHash mismatch: have %v, want %v", so0.CodeHash(), so1.CodeHash()) - } - if !bytes.Equal(so0.code, so1.code) { - t.Fatalf("Code mismatch: have %v, want %v", so0.code, so1.code) - } - - if len(so1.dirtyStorage) != len(so0.dirtyStorage) { - t.Errorf("Dirty storage size mismatch: have %d, want %d", len(so1.dirtyStorage), len(so0.dirtyStorage)) - } - for k, v := range so1.dirtyStorage { - if so0.dirtyStorage[k] != v { - t.Errorf("Dirty storage key %x mismatch: have %v, want %v", k, so0.dirtyStorage[k], v) - } - } - for k, v := range so0.dirtyStorage { - if so1.dirtyStorage[k] != v { - t.Errorf("Dirty storage key %x mismatch: have %v, want none.", k, v) - } - } - if len(so1.originStorage) != len(so0.originStorage) { - t.Errorf("Origin storage size mismatch: have %d, want %d", len(so1.originStorage), len(so0.originStorage)) - } - for k, v := range so1.originStorage { - if so0.originStorage[k] != v { - t.Errorf("Origin storage key %x mismatch: have %v, want %v", k, so0.originStorage[k], v) - } - } - for k, v := range so0.originStorage { - if so1.originStorage[k] != v { - t.Errorf("Origin storage key %x mismatch: have %v, want none.", k, v) - } + state.RevertToSnapshot(snap) + if state.Exist(addr) { + t.Error("Unexpected account after revert") } } diff --git a/core/state/statedb.go b/core/state/statedb.go index ab152dd18d..4a934fe82c 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -44,6 +44,26 @@ type revision struct { journalIndex int } +type mutationType int + +const ( + update mutationType = iota + deletion +) + +type mutation struct { + typ mutationType + applied bool +} + +func (m *mutation) copy() *mutation { + return &mutation{typ: m.typ, applied: m.applied} +} + +func (m *mutation) isDelete() bool { + return m.typ == deletion +} + // StateDB structs within the ethereum protocol are used to store anything // within the merkle trie. StateDBs take care of caching and storing // nested states. It's the general query interface to retrieve: @@ -75,12 +95,22 @@ type StateDB struct { accountsOrigin map[common.Address][]byte // The original value of mutated accounts in 'slim RLP' encoding storagesOrigin map[common.Address]map[common.Hash][]byte // The original value of mutated slots in prefix-zero trimmed rlp format - // This map holds 'live' objects, which will get modified while processing - // a state transition. - stateObjects map[common.Address]*stateObject - stateObjectsPending map[common.Address]struct{} // State objects finalized but not yet written to the trie - stateObjectsDirty map[common.Address]struct{} // State objects modified in the current execution - stateObjectsDestruct map[common.Address]*types.StateAccount // State objects destructed in the block along with its previous value + // This map holds 'live' objects, which will get modified while + // processing a state transition. + stateObjects map[common.Address]*stateObject + + // This map holds 'deleted' objects. An object with the same address + // might also occur in the 'stateObjects' map due to account + // resurrection. The account value is tracked as the original value + // before the transition. This map is populated at the transaction + // boundaries. + stateObjectsDestruct map[common.Address]*types.StateAccount + + // This map tracks the account mutations that occurred during the + // transition. Uncommitted mutations belonging to the same account + // can be merged into a single one which is equivalent from database's + // perspective. This map is populated at the transaction boundaries. + mutations map[common.Address]*mutation // DB error. // State objects are used by the consensus core and VM which are @@ -154,9 +184,8 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error) accountsOrigin: make(map[common.Address][]byte), storagesOrigin: make(map[common.Address]map[common.Hash][]byte), stateObjects: make(map[common.Address]*stateObject), - stateObjectsPending: make(map[common.Address]struct{}), - stateObjectsDirty: make(map[common.Address]struct{}), stateObjectsDestruct: make(map[common.Address]*types.StateAccount), + mutations: make(map[common.Address]*mutation), logs: make(map[common.Hash][]*types.Log), preimages: make(map[common.Hash][]byte), journal: newJournal(), @@ -472,8 +501,7 @@ func (s *StateDB) Selfdestruct6780(addr common.Address) { if stateObject == nil { return } - - if stateObject.created { + if stateObject.newContract { s.SelfDestruct(addr) } } @@ -552,24 +580,16 @@ func (s *StateDB) deleteStateObject(addr common.Address) { } // getStateObject retrieves a state object given by the address, returning nil if -// the object is not found or was deleted in this execution context. If you need -// to differentiate between non-existent/just-deleted, use getDeletedStateObject. +// the object is not found or was deleted in this execution context. func (s *StateDB) getStateObject(addr common.Address) *stateObject { - if obj := s.getDeletedStateObject(addr); obj != nil && !obj.deleted { - return obj - } - return nil -} - -// getDeletedStateObject is similar to getStateObject, but instead of returning -// nil for a deleted state object, it returns the actual object with the deleted -// flag set. This is needed by the state journal to revert to the correct s- -// destructed object instead of wiping all knowledge about the state object. -func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { // Prefer live objects if any is available if obj := s.stateObjects[addr]; obj != nil { return obj } + // Short circuit if the account is already destructed in this block. + if _, ok := s.stateObjectsDestruct[addr]; ok { + return nil + } // If no live objects are available, attempt to use snapshots var data *types.StateAccount if s.snap != nil { @@ -622,69 +642,40 @@ func (s *StateDB) setStateObject(object *stateObject) { // getOrNewStateObject retrieves a state object or create a new state object if nil. func (s *StateDB) getOrNewStateObject(addr common.Address) *stateObject { - stateObject := s.getStateObject(addr) - if stateObject == nil { - stateObject, _ = s.createObject(addr) + obj := s.getStateObject(addr) + if obj == nil { + obj = s.createObject(addr) } - return stateObject + return obj } -// createObject creates a new state object. If there is an existing account with -// the given address, it is overwritten and returned as the second return value. -func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject) { - prev = s.getDeletedStateObject(addr) // Note, prev might have been deleted, we need that! - newobj = newObject(s, addr, nil) - if prev == nil { - s.journal.append(createObjectChange{account: &addr}) - } else { - // The original account should be marked as destructed and all cached - // account and storage data should be cleared as well. Note, it must - // be done here, otherwise the destruction event of "original account" - // will be lost. - _, prevdestruct := s.stateObjectsDestruct[prev.address] - if !prevdestruct { - s.stateObjectsDestruct[prev.address] = prev.origin - } - // There may be some cached account/storage data already since IntermediateRoot - // will be called for each transaction before byzantium fork which will always - // cache the latest account/storage data. - prevAccount, ok := s.accountsOrigin[prev.address] - s.journal.append(resetObjectChange{ - account: &addr, - prev: prev, - prevdestruct: prevdestruct, - prevAccount: s.accounts[prev.addrHash], - prevStorage: s.storages[prev.addrHash], - prevAccountOriginExist: ok, - prevAccountOrigin: prevAccount, - prevStorageOrigin: s.storagesOrigin[prev.address], - }) - delete(s.accounts, prev.addrHash) - delete(s.storages, prev.addrHash) - delete(s.accountsOrigin, prev.address) - delete(s.storagesOrigin, prev.address) - } - s.setStateObject(newobj) - if prev != nil && !prev.deleted { - return newobj, prev - } - return newobj, nil +// createObject creates a new state object. The assumption is held there is no +// existing account with the given address, otherwise it will be silently overwritten. +func (s *StateDB) createObject(addr common.Address) *stateObject { + obj := newObject(s, addr, nil) + s.journal.append(createObjectChange{account: &addr}) + s.setStateObject(obj) + return obj } -// CreateAccount explicitly creates a state object. If a state object with the address -// already exists the balance is carried over to the new account. -// -// CreateAccount is called during the EVM CREATE operation. The situation might arise that -// a contract does the following: -// -// 1. sends funds to sha(account ++ (nonce + 1)) -// 2. tx_create(sha(account ++ nonce)) (note that this gets the address of 1) -// -// Carrying over the balance ensures that Ether doesn't disappear. +// CreateAccount explicitly creates a new state object, assuming that the +// account did not previously exist in the state. If the account already +// exists, this function will silently overwrite it which might lead to a +// consensus bug eventually. func (s *StateDB) CreateAccount(addr common.Address) { - newObj, prev := s.createObject(addr) - if prev != nil { - newObj.setBalance(prev.data.Balance) + s.createObject(addr) +} + +// CreateContract is used whenever a contract is created. This may be preceded +// by CreateAccount, but that is not required if it already existed in the +// state due to funds sent beforehand. +// This operation sets the 'newContract'-flag, which is required in order to +// correctly handle EIP-6780 'delete-in-same-transaction' logic. +func (s *StateDB) CreateContract(addr common.Address) { + obj := s.getStateObject(addr) + if !obj.newContract { + obj.newContract = true + s.journal.append(createContractChange{account: addr}) } } @@ -695,21 +686,25 @@ func (s *StateDB) Copy() *StateDB { state := &StateDB{ db: s.db, trie: s.db.CopyTrie(s.trie), + hasher: crypto.NewKeccakState(), originalRoot: s.originalRoot, accounts: copySet(s.accounts), storages: copy2DSet(s.storages), accountsOrigin: copySet(s.accountsOrigin), storagesOrigin: copy2DSet(s.storagesOrigin), - stateObjects: make(map[common.Address]*stateObject, len(s.journal.dirties)), - stateObjectsPending: make(map[common.Address]struct{}, len(s.stateObjectsPending)), - stateObjectsDirty: make(map[common.Address]struct{}, len(s.journal.dirties)), + stateObjects: make(map[common.Address]*stateObject, len(s.stateObjects)), stateObjectsDestruct: maps.Clone(s.stateObjectsDestruct), + mutations: make(map[common.Address]*mutation, len(s.mutations)), + dbErr: s.dbErr, refund: s.refund, + thash: s.thash, + txIndex: s.txIndex, logs: make(map[common.Hash][]*types.Log, len(s.logs)), logSize: s.logSize, preimages: maps.Clone(s.preimages), - journal: newJournal(), - hasher: crypto.NewKeccakState(), + journal: s.journal.copy(), + validRevisions: slices.Clone(s.validRevisions), + nextRevisionId: s.nextRevisionId, // In order for the block producer to be able to use and make additions // to the snapshot tree, we need to copy that as well. Otherwise, any @@ -718,39 +713,14 @@ func (s *StateDB) Copy() *StateDB { snaps: s.snaps, snap: s.snap, } - // Copy the dirty states, logs, and preimages - for addr := range s.journal.dirties { - // As documented [here](https://github.com/ethereum/go-ethereum/pull/16485#issuecomment-380438527), - // and in the Finalise-method, there is a case where an object is in the journal but not - // in the stateObjects: OOG after touch on ripeMD prior to Byzantium. Thus, we need to check for - // nil - if object, exist := s.stateObjects[addr]; exist { - // Even though the original object is dirty, we are not copying the journal, - // so we need to make sure that any side-effect the journal would have caused - // during a commit (or similar op) is already applied to the copy. - state.stateObjects[addr] = object.deepCopy(state) - - state.stateObjectsDirty[addr] = struct{}{} // Mark the copy dirty to force internal (code/state) commits - state.stateObjectsPending[addr] = struct{}{} // Mark the copy pending to force external (account) commits - } + // Deep copy cached state objects. + for addr, obj := range s.stateObjects { + state.stateObjects[addr] = obj.deepCopy(state) } - // Above, we don't copy the actual journal. This means that if the copy - // is copied, the loop above will be a no-op, since the copy's journal - // is empty. Thus, here we iterate over stateObjects, to enable copies - // of copies. - for addr := range s.stateObjectsPending { - if _, exist := state.stateObjects[addr]; !exist { - state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state) - } - state.stateObjectsPending[addr] = struct{}{} + // Deep copy the object state markers. + for addr, op := range s.mutations { + state.mutations[addr] = op.copy() } - for addr := range s.stateObjectsDirty { - if _, exist := state.stateObjects[addr]; !exist { - state.stateObjects[addr] = s.stateObjects[addr].deepCopy(state) - } - state.stateObjectsDirty[addr] = struct{}{} - } - // Deep copy the logs occurred in the scope of block for hash, logs := range s.logs { cpy := make([]*types.Log, len(logs)) @@ -760,7 +730,6 @@ func (s *StateDB) Copy() *StateDB { } state.logs[hash] = cpy } - // Do we need to copy the access list and transient storage? // In practice: No. At the start of a transaction, these two lists are empty. // In practice, we only ever copy state _between_ transactions/blocks, never @@ -825,7 +794,8 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { continue } if obj.selfDestructed || (deleteEmptyObjects && obj.empty()) { - obj.deleted = true + delete(s.stateObjects, obj.address) + s.markDelete(addr) // If ether was sent to account post-selfdestruct it is burnt. if bal := obj.Balance(); s.logger != nil && s.logger.OnBalanceChange != nil && obj.selfDestructed && bal.Sign() != 0 { @@ -846,11 +816,8 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) { delete(s.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect) } else { obj.finalise(true) // Prefetch slots in the background + s.markUpdate(addr) } - obj.created = false - s.stateObjectsPending[addr] = struct{}{} - s.stateObjectsDirty[addr] = struct{}{} - // At this point, also ship the address off to the precacher. The precacher // will start loading tries, and when the change is eventually committed, // the commit-phase will be a lot faster @@ -889,10 +856,14 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { // the account prefetcher. Instead, let's process all the storage updates // first, giving the account prefetches just a few more milliseconds of time // to pull useful data from disk. - for addr := range s.stateObjectsPending { - if obj := s.stateObjects[addr]; !obj.deleted { - obj.updateRoot() + for addr, op := range s.mutations { + if op.applied { + continue } + if op.isDelete() { + continue + } + s.stateObjects[addr].updateRoot() } // Now we're about to start to write changes to the trie. The trie is so far // _untouched_. We can check with the prefetcher, if it can give us a trie @@ -902,7 +873,6 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { s.trie = trie } } - usedAddrs := make([][]byte, 0, len(s.stateObjectsPending)) // Perform updates before deletions. This prevents resolution of unnecessary trie nodes // in circumstances similar to the following: // @@ -913,13 +883,21 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { // If the self-destruct is handled first, then `P` would be left with only one child, thus collapsed // into a shortnode. This requires `B` to be resolved from disk. // Whereas if the created node is handled first, then the collapse is avoided, and `B` is not resolved. - var deletedAddrs []common.Address - for addr := range s.stateObjectsPending { - if obj := s.stateObjects[addr]; !obj.deleted { - s.updateStateObject(obj) - s.AccountUpdated += 1 + var ( + usedAddrs [][]byte + deletedAddrs []common.Address + ) + for addr, op := range s.mutations { + if op.applied { + continue + } + op.applied = true + + if op.isDelete() { + deletedAddrs = append(deletedAddrs, addr) } else { - deletedAddrs = append(deletedAddrs, obj.address) + s.updateStateObject(s.stateObjects[addr]) + s.AccountUpdated += 1 } usedAddrs = append(usedAddrs, common.CopyBytes(addr[:])) // Copy needed for closure } @@ -930,9 +908,6 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { if prefetcher != nil { prefetcher.used(common.Hash{}, s.originalRoot, usedAddrs) } - if len(s.stateObjectsPending) > 0 { - s.stateObjectsPending = make(map[common.Address]struct{}) - } // Track the amount of time wasted on hashing the account trie defer func(start time.Time) { s.AccountHashes += time.Since(start) }(time.Now()) @@ -1176,11 +1151,12 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er return common.Hash{}, err } // Handle all state updates afterwards - for addr := range s.stateObjectsDirty { - obj := s.stateObjects[addr] - if obj.deleted { + for addr, op := range s.mutations { + if op.isDelete() { continue } + obj := s.stateObjects[addr] + // Write any contract code associated with the state object if obj.code != nil && obj.dirtyCode { rawdb.WriteCode(codeWriter, common.BytesToHash(obj.CodeHash()), obj.code) @@ -1280,7 +1256,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er s.storages = make(map[common.Hash]map[common.Hash][]byte) s.accountsOrigin = make(map[common.Address][]byte) s.storagesOrigin = make(map[common.Address]map[common.Hash][]byte) - s.stateObjectsDirty = make(map[common.Address]struct{}) + s.mutations = make(map[common.Address]*mutation) s.stateObjectsDestruct = make(map[common.Address]*types.StateAccount) return root, nil } @@ -1395,3 +1371,19 @@ func copy2DSet[k comparable](set map[k]map[common.Hash][]byte) map[k]map[common. } return copied } + +func (s *StateDB) markDelete(addr common.Address) { + if _, ok := s.mutations[addr]; !ok { + s.mutations[addr] = &mutation{} + } + s.mutations[addr].applied = false + s.mutations[addr].typ = deletion +} + +func (s *StateDB) markUpdate(addr common.Address) { + if _, ok := s.mutations[addr]; !ok { + s.mutations[addr] = &mutation{} + } + s.mutations[addr].applied = false + s.mutations[addr].typ = update +} diff --git a/core/state/statedb_fuzz_test.go b/core/state/statedb_fuzz_test.go index 65cf278108..6317681a7f 100644 --- a/core/state/statedb_fuzz_test.go +++ b/core/state/statedb_fuzz_test.go @@ -96,7 +96,9 @@ func newStateTestAction(addr common.Address, r *rand.Rand, index int) testAction { name: "CreateAccount", fn: func(a testAction, s *StateDB) { - s.CreateAccount(addr) + if !s.Exist(addr) { + s.CreateAccount(addr) + } }, }, { diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index edde6e8254..1a3eccfe10 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -225,6 +225,78 @@ func TestCopy(t *testing.T) { } } +// TestCopyWithDirtyJournal tests if Copy can correct create a equal copied +// stateDB with dirty journal present. +func TestCopyWithDirtyJournal(t *testing.T) { + db := NewDatabase(rawdb.NewMemoryDatabase()) + orig, _ := New(types.EmptyRootHash, db, nil) + + // Fill up the initial states + for i := byte(0); i < 255; i++ { + obj := orig.getOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(uint256.NewInt(uint64(i)), tracing.BalanceChangeUnspecified) + obj.data.Root = common.HexToHash("0xdeadbeef") + orig.updateStateObject(obj) + } + root, _ := orig.Commit(0, true) + orig, _ = New(root, db, nil) + + // modify all in memory without finalizing + for i := byte(0); i < 255; i++ { + obj := orig.getOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.SubBalance(uint256.NewInt(uint64(i)), tracing.BalanceChangeUnspecified) + orig.updateStateObject(obj) + } + cpy := orig.Copy() + + orig.Finalise(true) + for i := byte(0); i < 255; i++ { + root := orig.GetStorageRoot(common.BytesToAddress([]byte{i})) + if root != (common.Hash{}) { + t.Errorf("Unexpected storage root %x", root) + } + } + cpy.Finalise(true) + for i := byte(0); i < 255; i++ { + root := cpy.GetStorageRoot(common.BytesToAddress([]byte{i})) + if root != (common.Hash{}) { + t.Errorf("Unexpected storage root %x", root) + } + } + if cpy.IntermediateRoot(true) != orig.IntermediateRoot(true) { + t.Error("State is not equal after copy") + } +} + +// TestCopyObjectState creates an original state, S1, and makes a copy S2. +// It then proceeds to make changes to S1. Those changes are _not_ supposed +// to affect S2. This test checks that the copy properly deep-copies the objectstate +func TestCopyObjectState(t *testing.T) { + db := NewDatabase(rawdb.NewMemoryDatabase()) + orig, _ := New(types.EmptyRootHash, db, nil) + + // Fill up the initial states + for i := byte(0); i < 5; i++ { + obj := orig.getOrNewStateObject(common.BytesToAddress([]byte{i})) + obj.AddBalance(uint256.NewInt(uint64(i)), tracing.BalanceChangeUnspecified) + obj.data.Root = common.HexToHash("0xdeadbeef") + orig.updateStateObject(obj) + } + orig.Finalise(true) + cpy := orig.Copy() + for _, op := range cpy.mutations { + if have, want := op.applied, false; have != want { + t.Fatalf("Error in test itself, the 'done' flag should not be set before Commit, have %v want %v", have, want) + } + } + orig.Commit(0, true) + for _, op := range cpy.mutations { + if have, want := op.applied, false; have != want { + t.Fatalf("Error: original state affected copy, have %v want %v", have, want) + } + } +} + func TestSnapshotRandom(t *testing.T) { config := &quick.Config{MaxCount: 1000} err := quick.Check((*snapshotTest).run, config) @@ -308,7 +380,30 @@ func newTestAction(addr common.Address, r *rand.Rand) testAction { { name: "CreateAccount", fn: func(a testAction, s *StateDB) { - s.CreateAccount(addr) + if !s.Exist(addr) { + s.CreateAccount(addr) + } + }, + }, + { + name: "CreateContract", + fn: func(a testAction, s *StateDB) { + if !s.Exist(addr) { + s.CreateAccount(addr) + } + contractHash := s.GetCodeHash(addr) + emptyCode := contractHash == (common.Hash{}) || contractHash == types.EmptyCodeHash + storageRoot := s.GetStorageRoot(addr) + emptyStorage := storageRoot == (common.Hash{}) || storageRoot == types.EmptyRootHash + if s.GetNonce(addr) == 0 && emptyCode && emptyStorage { + s.CreateContract(addr) + // We also set some code here, to prevent the + // CreateContract action from being performed twice in a row, + // which would cause a difference in state when unrolling + // the journal. (CreateContact assumes created was false prior to + // invocation, and the journal rollback sets it to false). + s.SetCode(addr, []byte{1}) + } }, }, { @@ -709,18 +804,19 @@ func TestCopyCopyCommitCopy(t *testing.T) { } } -// TestCommitCopy tests the copy from a committed state is not functional. +// TestCommitCopy tests the copy from a committed state is not fully functional. func TestCommitCopy(t *testing.T) { - state, _ := New(types.EmptyRootHash, NewDatabase(rawdb.NewMemoryDatabase()), nil) + db := NewDatabase(rawdb.NewMemoryDatabase()) + state, _ := New(types.EmptyRootHash, db, nil) // Create an account and check if the retrieved balance is correct addr := common.HexToAddress("0xaffeaffeaffeaffeaffeaffeaffeaffeaffeaffe") - skey := common.HexToHash("aaa") - sval := common.HexToHash("bbb") + skey1, skey2 := common.HexToHash("a1"), common.HexToHash("a2") + sval1, sval2 := common.HexToHash("b1"), common.HexToHash("b2") state.SetBalance(addr, uint256.NewInt(42), tracing.BalanceChangeUnspecified) // Change the account trie state.SetCode(addr, []byte("hello")) // Change an external metadata - state.SetState(addr, skey, sval) // Change the storage trie + state.SetState(addr, skey1, sval1) // Change the storage trie if balance := state.GetBalance(addr); balance.Cmp(uint256.NewInt(42)) != 0 { t.Fatalf("initial balance mismatch: have %v, want %v", balance, 42) @@ -728,25 +824,38 @@ func TestCommitCopy(t *testing.T) { if code := state.GetCode(addr); !bytes.Equal(code, []byte("hello")) { t.Fatalf("initial code mismatch: have %x, want %x", code, []byte("hello")) } - if val := state.GetState(addr, skey); val != sval { - t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval) + if val := state.GetState(addr, skey1); val != sval1 { + t.Fatalf("initial non-committed storage slot mismatch: have %x, want %x", val, sval1) } - if val := state.GetCommittedState(addr, skey); val != (common.Hash{}) { + if val := state.GetCommittedState(addr, skey1); val != (common.Hash{}) { t.Fatalf("initial committed storage slot mismatch: have %x, want %x", val, common.Hash{}) } - // Copy the committed state database, the copied one is not functional. - state.Commit(0, true) + root, _ := state.Commit(0, true) + + state, _ = New(root, db, nil) + state.SetState(addr, skey2, sval2) + state.Commit(1, true) + + // Copy the committed state database, the copied one is not fully functional. copied := state.Copy() - if balance := copied.GetBalance(addr); balance.Cmp(uint256.NewInt(0)) != 0 { + if balance := copied.GetBalance(addr); balance.Cmp(uint256.NewInt(42)) != 0 { t.Fatalf("unexpected balance: have %v", balance) } - if code := copied.GetCode(addr); code != nil { + if code := copied.GetCode(addr); !bytes.Equal(code, []byte("hello")) { t.Fatalf("unexpected code: have %x", code) } - if val := copied.GetState(addr, skey); val != (common.Hash{}) { + // Miss slots because of non-functional trie after commit + if val := copied.GetState(addr, skey1); val != (common.Hash{}) { + t.Fatalf("unexpected storage slot: have %x", sval1) + } + if val := copied.GetCommittedState(addr, skey1); val != (common.Hash{}) { t.Fatalf("unexpected storage slot: have %x", val) } - if val := copied.GetCommittedState(addr, skey); val != (common.Hash{}) { + // Slots cached in the stateDB, available after commit + if val := copied.GetState(addr, skey2); val != sval2 { + t.Fatalf("unexpected storage slot: have %x", sval1) + } + if val := copied.GetCommittedState(addr, skey2); val != sval2 { t.Fatalf("unexpected storage slot: have %x", val) } if !errors.Is(copied.Error(), trie.ErrCommitted) { @@ -1103,40 +1212,6 @@ func TestStateDBTransientStorage(t *testing.T) { } } -func TestResetObject(t *testing.T) { - var ( - disk = rawdb.NewMemoryDatabase() - tdb = triedb.NewDatabase(disk, nil) - db = NewDatabaseWithNodeDB(disk, tdb) - snaps, _ = snapshot.New(snapshot.Config{CacheSize: 10}, disk, tdb, types.EmptyRootHash) - state, _ = New(types.EmptyRootHash, db, snaps) - addr = common.HexToAddress("0x1") - slotA = common.HexToHash("0x1") - slotB = common.HexToHash("0x2") - ) - // Initialize account with balance and storage in first transaction. - state.SetBalance(addr, uint256.NewInt(1), tracing.BalanceChangeUnspecified) - state.SetState(addr, slotA, common.BytesToHash([]byte{0x1})) - state.IntermediateRoot(true) - - // Reset account and mutate balance and storages - state.CreateAccount(addr) - state.SetBalance(addr, uint256.NewInt(2), tracing.BalanceChangeUnspecified) - state.SetState(addr, slotB, common.BytesToHash([]byte{0x2})) - root, _ := state.Commit(0, true) - - // Ensure the original account is wiped properly - snap := snaps.Snapshot(root) - slot, _ := snap.Storage(crypto.Keccak256Hash(addr.Bytes()), crypto.Keccak256Hash(slotA.Bytes())) - if len(slot) != 0 { - t.Fatalf("Unexpected storage slot") - } - slot, _ = snap.Storage(crypto.Keccak256Hash(addr.Bytes()), crypto.Keccak256Hash(slotB.Bytes())) - if !bytes.Equal(slot, []byte{0x2}) { - t.Fatalf("Unexpected storage slot value %v", slot) - } -} - func TestDeleteStorage(t *testing.T) { var ( disk = rawdb.NewMemoryDatabase() diff --git a/core/vm/evm.go b/core/vm/evm.go index 045506a1fd..c18353a973 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -436,14 +436,15 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, return nil, common.Address{}, gas, ErrNonceUintOverflow } evm.StateDB.SetNonce(caller.Address(), nonce+1) - // We add this to the access list _before_ taking a snapshot. Even if the creation fails, - // the access-list change should not be rolled back + + // We add this to the access list _before_ taking a snapshot. Even if the + // creation fails, the access-list change should not be rolled back. if evm.chainRules.IsBerlin { evm.StateDB.AddAddressToAccessList(address) } // Ensure there's no existing contract already at the designated address. // Account is regarded as existent if any of these three conditions is met: - // - the nonce is nonzero + // - the nonce is non-zero // - the code is non-empty // - the storage is non-empty contractHash := evm.StateDB.GetCodeHash(address) @@ -456,9 +457,19 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, } return nil, common.Address{}, 0, ErrContractAddressCollision } - // Create a new account on the state + // Create a new account on the state only if the object was not present. + // It might be possible the contract code is deployed to a pre-existent + // account with non-zero balance. snapshot := evm.StateDB.Snapshot() - evm.StateDB.CreateAccount(address) + if !evm.StateDB.Exist(address) { + evm.StateDB.CreateAccount(address) + } + // CreateContract means that regardless of whether the account previously existed + // in the state trie or not, it _now_ becomes created as a _contract_ account. + // This is performed _prior_ to executing the initcode, since the initcode + // acts inside that account. + evm.StateDB.CreateContract(address) + if evm.chainRules.IsEIP158 { evm.StateDB.SetNonce(address, 1) } diff --git a/core/vm/interface.go b/core/vm/interface.go index 30742e96de..774360a08e 100644 --- a/core/vm/interface.go +++ b/core/vm/interface.go @@ -29,6 +29,7 @@ import ( // StateDB is an EVM database for full state querying. type StateDB interface { CreateAccount(common.Address) + CreateContract(common.Address) SubBalance(common.Address, *uint256.Int, tracing.BalanceChangeReason) AddBalance(common.Address, *uint256.Int, tracing.BalanceChangeReason) diff --git a/tests/block_test.go b/tests/block_test.go index 43e3d99b3e..1ba84f5f24 100644 --- a/tests/block_test.go +++ b/tests/block_test.go @@ -64,14 +64,6 @@ func TestExecutionSpecBlocktests(t *testing.T) { } bt := new(testMatcher) - // These tests fail as of https://github.com/ethereum/go-ethereum/pull/28666, since we - // no longer delete "leftover storage" when deploying a contract. - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/self_destructing_initcode_create_tx.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/self_destructing_initcode.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/recreate_self_destructed_contract_different_txs.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/delegatecall_from_new_contract_to_pre_existing_contract.json`) - bt.skipLoad(`^cancun/eip6780_selfdestruct/selfdestruct/create_selfdestruct_same_tx.json`) - bt.walk(t, executionSpecBlockchainTestDir, func(t *testing.T, name string, test *BlockTest) { execBlockTest(t, bt, test) }) From ac21f9bfb5154854e241928840b3978d801eac8c Mon Sep 17 00:00:00 2001 From: qcrao Date: Wed, 24 Apr 2024 20:04:20 +0800 Subject: [PATCH 14/15] trie: preallocate capacity for fields slice (#29614) trie: Preallocate capacity for fields slice --- trie/trienode/node.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/trie/trienode/node.go b/trie/trienode/node.go index 95315c2e9a..055db8822e 100644 --- a/trie/trienode/node.go +++ b/trie/trienode/node.go @@ -78,7 +78,7 @@ func NewNodeSet(owner common.Hash) *NodeSet { // ForEachWithOrder iterates the nodes with the order from bottom to top, // right to left, nodes with the longest path will be iterated first. func (set *NodeSet) ForEachWithOrder(callback func(path string, n *Node)) { - var paths []string + paths := make([]string, 0, len(set.Nodes)) for path := range set.Nodes { paths = append(paths, path) } @@ -133,7 +133,7 @@ func (set *NodeSet) Size() (int, int) { // Hashes returns the hashes of all updated nodes. TODO(rjl493456442) how can // we get rid of it? func (set *NodeSet) Hashes() []common.Hash { - var ret []common.Hash + ret := make([]common.Hash, 0, len(set.Nodes)) for _, node := range set.Nodes { ret = append(ret, node.Hash) } From 7362691479df9eb7d76c8b968ed8f440372fab6e Mon Sep 17 00:00:00 2001 From: Aaron Chen Date: Wed, 24 Apr 2024 20:27:58 +0800 Subject: [PATCH 15/15] trie, consensus/clique: use maps.Clone (#29616) --- consensus/clique/snapshot.go | 23 ++++++----------------- trie/tracer.go | 18 +++++------------- 2 files changed, 11 insertions(+), 30 deletions(-) diff --git a/consensus/clique/snapshot.go b/consensus/clique/snapshot.go index 8ff0b3a70f..d0b15e9489 100644 --- a/consensus/clique/snapshot.go +++ b/consensus/clique/snapshot.go @@ -19,6 +19,7 @@ package clique import ( "bytes" "encoding/json" + "maps" "slices" "time" @@ -108,28 +109,16 @@ func (s *Snapshot) store(db ethdb.Database) error { // copy creates a deep copy of the snapshot, though not the individual votes. func (s *Snapshot) copy() *Snapshot { - cpy := &Snapshot{ + return &Snapshot{ config: s.config, sigcache: s.sigcache, Number: s.Number, Hash: s.Hash, - Signers: make(map[common.Address]struct{}), - Recents: make(map[uint64]common.Address), - Votes: make([]*Vote, len(s.Votes)), - Tally: make(map[common.Address]Tally), + Signers: maps.Clone(s.Signers), + Recents: maps.Clone(s.Recents), + Votes: slices.Clone(s.Votes), + Tally: maps.Clone(s.Tally), } - for signer := range s.Signers { - cpy.Signers[signer] = struct{}{} - } - for block, signer := range s.Recents { - cpy.Recents[block] = signer - } - for address, tally := range s.Tally { - cpy.Tally[address] = tally - } - copy(cpy.Votes, s.Votes) - - return cpy } // validVote returns whether it makes sense to cast the specified vote in the diff --git a/trie/tracer.go b/trie/tracer.go index 5786af4d3e..90b9666f0b 100644 --- a/trie/tracer.go +++ b/trie/tracer.go @@ -17,6 +17,8 @@ package trie import ( + "maps" + "github.com/ethereum/go-ethereum/common" ) @@ -92,23 +94,13 @@ func (t *tracer) reset() { // copy returns a deep copied tracer instance. func (t *tracer) copy() *tracer { - var ( - inserts = make(map[string]struct{}) - deletes = make(map[string]struct{}) - accessList = make(map[string][]byte) - ) - for path := range t.inserts { - inserts[path] = struct{}{} - } - for path := range t.deletes { - deletes[path] = struct{}{} - } + accessList := make(map[string][]byte, len(t.accessList)) for path, blob := range t.accessList { accessList[path] = common.CopyBytes(blob) } return &tracer{ - inserts: inserts, - deletes: deletes, + inserts: maps.Clone(t.inserts), + deletes: maps.Clone(t.deletes), accessList: accessList, } }