From 896cafdcbbb55a472cbecea53c275f782f3db961 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Wed, 20 May 2020 11:37:01 -0400 Subject: [PATCH 01/29] debugging utxo error --- main/params.go | 2 +- scripts/ansible/cascade-inventory.yml | 4 +- snow/engine/avalanche/bootstrapper.go | 32 ++++--- snow/engine/avalanche/metrics.go | 34 +++---- snow/engine/avalanche/transitive.go | 2 +- snow/engine/avalanche/tx_job.go | 30 ++++++- snow/engine/avalanche/vertex_job.go | 4 + snow/engine/common/queue/jobs.go | 6 ++ snow/engine/common/queue/jobs_test.go | 123 +++++++++++++++++++++++++- vms/avm/unique_tx.go | 17 ++-- vms/components/ava/state.go | 3 +- 11 files changed, 213 insertions(+), 44 deletions(-) diff --git a/main/params.go b/main/params.go index c18a9375668..c815b712da8 100644 --- a/main/params.go +++ b/main/params.go @@ -29,7 +29,7 @@ import ( ) const ( - dbVersion = "v0.2.0" + dbVersion = "v0.2.-5" ) // Results of parsing the CLI diff --git a/scripts/ansible/cascade-inventory.yml b/scripts/ansible/cascade-inventory.yml index a115a9073fa..283ddfebbff 100755 --- a/scripts/ansible/cascade-inventory.yml +++ b/scripts/ansible/cascade-inventory.yml @@ -43,8 +43,8 @@ borealis_bootstrap: borealis_node: hosts: - node1: - ansible_host: 34.207.133.167 + # node1: + # ansible_host: 34.207.133.167 node2: ansible_host: 107.23.241.199 node3: diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index 304172f2e32..75e8fbadff5 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -43,14 +43,16 @@ func (b *bootstrapper) Initialize(config BootstrapConfig) { b.BootstrapConfig = config b.VtxBlocked.SetParser(&vtxParser{ - numAccepted: b.numBootstrappedVtx, - numDropped: b.numDroppedVtx, + log: config.Context.Log, + numAccepted: b.numBSVtx, + numDropped: b.numBSDroppedVtx, state: b.State, }) b.TxBlocked.SetParser(&txParser{ - numAccepted: b.numBootstrappedTx, - numDropped: b.numDroppedTx, + log: config.Context.Log, + numAccepted: b.numBSTx, + numDropped: b.numBSDroppedTx, vm: b.VM, }) @@ -155,7 +157,7 @@ func (b *bootstrapper) sendRequest(vtxID ids.ID) { b.pending.Add(vtxID) b.BootstrapConfig.Sender.Get(validatorID, b.RequestID, vtxID) - b.numPendingRequests.Set(float64(b.pending.Len())) + b.numBSPendingRequests.Set(float64(b.pending.Len())) } func (b *bootstrapper) addVertex(vtx avalanche.Vertex) { @@ -182,19 +184,21 @@ func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { b.pending.Remove(vtxID) if err := b.VtxBlocked.Push(&vertexJob{ - numAccepted: b.numBootstrappedVtx, - numDropped: b.numDroppedVtx, + log: b.BootstrapConfig.Context.Log, + numAccepted: b.numBSVtx, + numDropped: b.numBSDroppedVtx, vtx: vtx, }); err == nil { - b.numBlockedVtx.Inc() + b.numBSBlockedVtx.Inc() } for _, tx := range vtx.Txs() { if err := b.TxBlocked.Push(&txJob{ - numAccepted: b.numBootstrappedVtx, - numDropped: b.numDroppedVtx, + log: b.BootstrapConfig.Context.Log, + numAccepted: b.numBSTx, + numDropped: b.numBSDroppedTx, tx: tx, }); err == nil { - b.numBlockedTx.Inc() + b.numBSBlockedTx.Inc() } } @@ -207,7 +211,7 @@ func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { } numPending := b.pending.Len() - b.numPendingRequests.Set(float64(numPending)) + b.numBSPendingRequests.Set(float64(numPending)) } func (b *bootstrapper) finish() { @@ -215,8 +219,8 @@ func (b *bootstrapper) finish() { return } - b.executeAll(b.TxBlocked, b.numBlockedTx) - b.executeAll(b.VtxBlocked, b.numBlockedVtx) + b.executeAll(b.TxBlocked, b.numBSBlockedTx) + b.executeAll(b.VtxBlocked, b.numBSBlockedVtx) // Start consensus b.onFinished() diff --git a/snow/engine/avalanche/metrics.go b/snow/engine/avalanche/metrics.go index c1a15948699..021fe382d22 100644 --- a/snow/engine/avalanche/metrics.go +++ b/snow/engine/avalanche/metrics.go @@ -10,52 +10,52 @@ import ( ) type metrics struct { - numPendingRequests, numBlockedVtx, numBlockedTx prometheus.Gauge - numBootstrappedVtx, numDroppedVtx, - numBootstrappedTx, numDroppedTx prometheus.Counter + numBSPendingRequests, numBSBlockedVtx, numBSBlockedTx prometheus.Gauge + numBSVtx, numBSDroppedVtx, + numBSTx, numBSDroppedTx prometheus.Counter numPolls, numVtxRequests, numTxRequests, numPendingVtx prometheus.Gauge } // Initialize implements the Engine interface func (m *metrics) Initialize(log logging.Logger, namespace string, registerer prometheus.Registerer) { - m.numPendingRequests = prometheus.NewGauge( + m.numBSPendingRequests = prometheus.NewGauge( prometheus.GaugeOpts{ Namespace: namespace, Name: "av_bs_vtx_requests", Help: "Number of pending bootstrap vertex requests", }) - m.numBlockedVtx = prometheus.NewGauge( + m.numBSBlockedVtx = prometheus.NewGauge( prometheus.GaugeOpts{ Namespace: namespace, Name: "av_bs_blocked_vts", Help: "Number of blocked bootstrap vertices", }) - m.numBlockedTx = prometheus.NewGauge( + m.numBSBlockedTx = prometheus.NewGauge( prometheus.GaugeOpts{ Namespace: namespace, Name: "av_bs_blocked_txs", Help: "Number of blocked bootstrap txs", }) - m.numBootstrappedVtx = prometheus.NewCounter( + m.numBSVtx = prometheus.NewCounter( prometheus.CounterOpts{ Namespace: namespace, Name: "av_bs_accepted_vts", Help: "Number of accepted vertices", }) - m.numDroppedVtx = prometheus.NewCounter( + m.numBSDroppedVtx = prometheus.NewCounter( prometheus.CounterOpts{ Namespace: namespace, Name: "av_bs_dropped_vts", Help: "Number of dropped vertices", }) - m.numBootstrappedTx = prometheus.NewCounter( + m.numBSTx = prometheus.NewCounter( prometheus.CounterOpts{ Namespace: namespace, Name: "av_bs_accepted_txs", Help: "Number of accepted txs", }) - m.numDroppedTx = prometheus.NewCounter( + m.numBSDroppedTx = prometheus.NewCounter( prometheus.CounterOpts{ Namespace: namespace, Name: "av_bs_dropped_txs", @@ -86,25 +86,25 @@ func (m *metrics) Initialize(log logging.Logger, namespace string, registerer pr Help: "Number of blocked vertices", }) - if err := registerer.Register(m.numPendingRequests); err != nil { + if err := registerer.Register(m.numBSPendingRequests); err != nil { log.Error("Failed to register av_bs_vtx_requests statistics due to %s", err) } - if err := registerer.Register(m.numBlockedVtx); err != nil { + if err := registerer.Register(m.numBSBlockedVtx); err != nil { log.Error("Failed to register av_bs_blocked_vts statistics due to %s", err) } - if err := registerer.Register(m.numBlockedTx); err != nil { + if err := registerer.Register(m.numBSBlockedTx); err != nil { log.Error("Failed to register av_bs_blocked_txs statistics due to %s", err) } - if err := registerer.Register(m.numBootstrappedVtx); err != nil { + if err := registerer.Register(m.numBSVtx); err != nil { log.Error("Failed to register av_bs_accepted_vts statistics due to %s", err) } - if err := registerer.Register(m.numDroppedVtx); err != nil { + if err := registerer.Register(m.numBSDroppedVtx); err != nil { log.Error("Failed to register av_bs_dropped_vts statistics due to %s", err) } - if err := registerer.Register(m.numBootstrappedTx); err != nil { + if err := registerer.Register(m.numBSTx); err != nil { log.Error("Failed to register av_bs_accepted_txs statistics due to %s", err) } - if err := registerer.Register(m.numDroppedTx); err != nil { + if err := registerer.Register(m.numBSDroppedTx); err != nil { log.Error("Failed to register av_bs_dropped_txs statistics due to %s", err) } if err := registerer.Register(m.numPolls); err != nil { diff --git a/snow/engine/avalanche/transitive.go b/snow/engine/avalanche/transitive.go index be4bac91a05..c8a5d33cf38 100644 --- a/snow/engine/avalanche/transitive.go +++ b/snow/engine/avalanche/transitive.go @@ -333,7 +333,7 @@ func (t *Transitive) insert(vtx avalanche.Vertex) { // Track performance statistics t.numVtxRequests.Set(float64(t.vtxReqs.Len())) t.numTxRequests.Set(float64(t.missingTxs.Len())) - t.numBlockedVtx.Set(float64(t.pending.Len())) + t.numPendingVtx.Set(float64(t.pending.Len())) } func (t *Transitive) batch(txs []snowstorm.Tx, force, empty bool) { diff --git a/snow/engine/avalanche/tx_job.go b/snow/engine/avalanche/tx_job.go index f0ffe70940d..f53333078dd 100644 --- a/snow/engine/avalanche/tx_job.go +++ b/snow/engine/avalanche/tx_job.go @@ -4,15 +4,19 @@ package avalanche import ( + "encoding/json" + "github.com/prometheus/client_golang/prometheus" "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/choices" "github.com/ava-labs/gecko/snow/consensus/snowstorm" "github.com/ava-labs/gecko/snow/engine/common/queue" + "github.com/ava-labs/gecko/utils/logging" ) type txParser struct { + log logging.Logger numAccepted, numDropped prometheus.Counter vm DAGVM } @@ -23,6 +27,7 @@ func (p *txParser) Parse(txBytes []byte) (queue.Job, error) { return nil, err } return &txJob{ + log: p.log, numAccepted: p.numAccepted, numDropped: p.numDropped, tx: tx, @@ -30,6 +35,7 @@ func (p *txParser) Parse(txBytes []byte) (queue.Job, error) { } type txJob struct { + log logging.Logger numAccepted, numDropped prometheus.Counter tx snowstorm.Tx } @@ -44,6 +50,11 @@ func (t *txJob) MissingDependencies() ids.Set { } return missing } + +var ( + accepted ids.Set +) + func (t *txJob) Execute() { if t.MissingDependencies().Len() != 0 { t.numDropped.Inc() @@ -54,7 +65,24 @@ func (t *txJob) Execute() { case choices.Unknown, choices.Rejected: t.numDropped.Inc() case choices.Processing: - t.tx.Verify() + if err := t.tx.Verify(); err != nil { + tx, _ := json.MarshalIndent(t.tx, "", " ") + parents, _ := json.MarshalIndent(t.tx.Dependencies(), "", " ") + t.log.Warn("transaction %s failed verification during bootstrapping due to %s\n"+ + "Tx: %s\n"+ + "Inputs: %s\n"+ + "Dependencies: %s", + t.tx.ID(), err, + tx, + t.tx.InputIDs(), + parents) + } + + if accepted.Overlaps(t.tx.InputIDs()) { + t.log.Fatal("Bootstrapping attempted to accept a double spend") + accepted.Union(t.tx.InputIDs()) + } + t.tx.Accept() t.numAccepted.Inc() } diff --git a/snow/engine/avalanche/vertex_job.go b/snow/engine/avalanche/vertex_job.go index 2de19a0a9f7..19386b70536 100644 --- a/snow/engine/avalanche/vertex_job.go +++ b/snow/engine/avalanche/vertex_job.go @@ -10,9 +10,11 @@ import ( "github.com/ava-labs/gecko/snow/choices" "github.com/ava-labs/gecko/snow/consensus/avalanche" "github.com/ava-labs/gecko/snow/engine/common/queue" + "github.com/ava-labs/gecko/utils/logging" ) type vtxParser struct { + log logging.Logger numAccepted, numDropped prometheus.Counter state State } @@ -23,6 +25,7 @@ func (p *vtxParser) Parse(vtxBytes []byte) (queue.Job, error) { return nil, err } return &vertexJob{ + log: p.log, numAccepted: p.numAccepted, numDropped: p.numDropped, vtx: vtx, @@ -30,6 +33,7 @@ func (p *vtxParser) Parse(vtxBytes []byte) (queue.Job, error) { } type vertexJob struct { + log logging.Logger numAccepted, numDropped prometheus.Counter vtx avalanche.Vertex } diff --git a/snow/engine/common/queue/jobs.go b/snow/engine/common/queue/jobs.go index 0a5c45110cb..0456a463b5d 100644 --- a/snow/engine/common/queue/jobs.go +++ b/snow/engine/common/queue/jobs.go @@ -128,6 +128,12 @@ func (j *Jobs) push(job Job) error { } func (j *Jobs) block(job Job, deps ids.Set) error { + if has, err := j.state.HasJob(j.db, job.ID()); err != nil { + return err + } else if has { + return errDuplicate + } + if err := j.state.SetJob(j.db, job); err != nil { return err } diff --git a/snow/engine/common/queue/jobs_test.go b/snow/engine/common/queue/jobs_test.go index ba2d4d16e5c..30b83a5c862 100644 --- a/snow/engine/common/queue/jobs_test.go +++ b/snow/engine/common/queue/jobs_test.go @@ -5,12 +5,14 @@ package queue import ( "bytes" + "errors" "testing" "github.com/ava-labs/gecko/database/memdb" "github.com/ava-labs/gecko/ids" ) +// Test that creating a new queue can be created and that it is initially empty. func TestNew(t *testing.T) { parser := &TestParser{T: t} db := memdb.New() @@ -29,6 +31,8 @@ func TestNew(t *testing.T) { } } +// Test that a job can be added to a queue, and then the job can be removed from +// the queue after a shutdown. func TestPushPop(t *testing.T) { parser := &TestParser{T: t} db := memdb.New() @@ -94,6 +98,8 @@ func TestPushPop(t *testing.T) { } } +// Test that executing a job will cause a dependent job to be placed on to the +// ready queue func TestExecute(t *testing.T) { parser := &TestParser{T: t} db := memdb.New() @@ -116,7 +122,7 @@ func TestExecute(t *testing.T) { BytesF: func() []byte { return []byte{0} }, } - id1 := ids.Empty.Prefix(0) + id1 := ids.Empty.Prefix(1) executed1 := new(bool) job1 := &TestJob{ T: t, @@ -182,7 +188,8 @@ func TestExecute(t *testing.T) { } } -func TestDuplicatedPush(t *testing.T) { +// Test that a job that is ready to be executed can only be added once +func TestDuplicatedExecutablePush(t *testing.T) { parser := &TestParser{T: t} db := memdb.New() @@ -250,3 +257,115 @@ func TestDuplicatedPush(t *testing.T) { t.Fatalf("Shouldn't have a container ready to pop") } } + +// Test that a job that isn't ready to be executed can only be added once +func TestDuplicatedNotExecutablePush(t *testing.T) { + parser := &TestParser{T: t} + db := memdb.New() + + jobs, err := New(db) + if err != nil { + t.Fatal(err) + } + + jobs.SetParser(parser) + + id0 := ids.Empty.Prefix(0) + id1 := ids.Empty.Prefix(1) + job1 := &TestJob{ + T: t, + + IDF: func() ids.ID { return id1 }, + MissingDependenciesF: func() ids.Set { + s := ids.Set{} + s.Add(id0) + return s + }, + ExecuteF: func() {}, + BytesF: func() []byte { return []byte{1} }, + } + job0 := &TestJob{ + T: t, + + IDF: func() ids.ID { return id0 }, + MissingDependenciesF: func() ids.Set { return ids.Set{} }, + ExecuteF: func() { + job1.MissingDependenciesF = func() ids.Set { return ids.Set{} } + }, + BytesF: func() []byte { return []byte{0} }, + } + + if err := jobs.Push(job1); err != nil { + t.Fatal(err) + } + + if err := jobs.Push(job1); err == nil { + t.Fatalf("should have errored on pushing a duplicate job") + } + + if err := jobs.Commit(); err != nil { + t.Fatal(err) + } + + jobs, err = New(db) + if err != nil { + t.Fatal(err) + } + + jobs.SetParser(parser) + + if err := jobs.Push(job0); err != nil { + t.Fatal(err) + } + + if hasNext, err := jobs.HasNext(); err != nil { + t.Fatal(err) + } else if !hasNext { + t.Fatalf("Should have a container ready to pop") + } + + parser.ParseF = func(b []byte) (Job, error) { + if bytes.Equal(b, []byte{0}) { + return job0, nil + } + if bytes.Equal(b, []byte{1}) { + return job1, nil + } + t.Fatalf("Unknown job") + return nil, errors.New("Unknown job") + } + + returnedBlockable, err := jobs.Pop() + if err != nil { + t.Fatal(err) + } + + if returnedBlockable != job0 { + t.Fatalf("Returned wrong job") + } + + if err := jobs.Execute(job0); err != nil { + t.Fatal(err) + } + + if hasNext, err := jobs.HasNext(); err != nil { + t.Fatal(err) + } else if !hasNext { + t.Fatalf("Should have a container ready to pop") + } + + returnedBlockable, err = jobs.Pop() + if err != nil { + t.Fatal(err) + } + + if returnedBlockable != job1 { + t.Fatalf("Returned wrong job") + } + + if hasNext, err := jobs.HasNext(); err != nil { + t.Fatal(err) + } else if hasNext { + t.Fatalf("Shouldn't have a container ready to pop") + } +} diff --git a/vms/avm/unique_tx.go b/vms/avm/unique_tx.go index 788ef523465..68bdca87e27 100644 --- a/vms/avm/unique_tx.go +++ b/vms/avm/unique_tx.go @@ -100,13 +100,13 @@ func (tx *UniqueTx) ID() ids.ID { return tx.txID } // Accept is called when the transaction was finalized as accepted by consensus func (tx *UniqueTx) Accept() { - defer tx.vm.db.Abort() - - if err := tx.setStatus(choices.Accepted); err != nil { - tx.vm.ctx.Log.Error("Failed to accept tx %s due to %s", tx.txID, err) + if s := tx.Status(); s != choices.Processing { + tx.vm.ctx.Log.Error("Failed to accept tx %s because the tx is in state %s", tx.txID, s) return } + defer tx.vm.db.Abort() + // Remove spent utxos for _, utxo := range tx.InputUTXOs() { if utxo.Symbolic() { @@ -123,20 +123,27 @@ func (tx *UniqueTx) Accept() { // Add new utxos for _, utxo := range tx.UTXOs() { if err := tx.vm.state.FundUTXO(utxo); err != nil { - tx.vm.ctx.Log.Error("Failed to fund utxo %s due to %s", utxoID, err) + tx.vm.ctx.Log.Error("Failed to fund utxo %s due to %s", utxo.InputID(), err) return } } + if err := tx.setStatus(choices.Accepted); err != nil { + tx.vm.ctx.Log.Error("Failed to accept tx %s due to %s", tx.txID, err) + return + } + txID := tx.ID() commitBatch, err := tx.vm.db.CommitBatch() if err != nil { tx.vm.ctx.Log.Error("Failed to calculate CommitBatch for %s due to %s", txID, err) + tx.setStatus(choices.Processing) return } if err := tx.ExecuteWithSideEffects(tx.vm, commitBatch); err != nil { tx.vm.ctx.Log.Error("Failed to commit accept %s due to %s", txID, err) + tx.setStatus(choices.Processing) return } diff --git a/vms/components/ava/state.go b/vms/components/ava/state.go index 627cc5650af..93462039ed5 100644 --- a/vms/components/ava/state.go +++ b/vms/components/ava/state.go @@ -5,6 +5,7 @@ package ava import ( "errors" + "fmt" "github.com/ava-labs/gecko/cache" "github.com/ava-labs/gecko/database" @@ -146,7 +147,7 @@ func (s *State) SetIDs(id ids.ID, idSlice []ids.ID) error { bytes, err := s.Codec.Marshal(idSlice) if err != nil { - return err + return fmt.Errorf("failed to marshal an ID array due to %w", err) } s.Cache.Put(id, idSlice) From 537e72714fe0dd70bc5feefdb4494739d5593ef3 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Tue, 26 May 2020 13:25:34 -0400 Subject: [PATCH 02/29] Added returned errors to Accept and Reject in decidables --- snow/choices/decision.go | 4 +- snow/consensus/avalanche/consensus.go | 10 ++-- snow/consensus/avalanche/topological.go | 54 ++++++++++++------- snow/consensus/avalanche/vertex_test.go | 4 +- snow/consensus/snowman/block_test.go | 11 ++-- snow/consensus/snowman/consensus.go | 7 +-- snow/consensus/snowman/topological.go | 46 ++++++++++------ snow/consensus/snowstorm/consensus.go | 9 ++-- snow/consensus/snowstorm/directed.go | 55 ++++++++++++++------ snow/consensus/snowstorm/input.go | 43 ++++++++++----- snow/consensus/snowstorm/test_tx.go | 4 +- snow/engine/avalanche/engine_test.go | 4 +- snow/engine/avalanche/state/unique_vertex.go | 8 +-- snow/engine/snowman/engine_test.go | 4 +- vms/avm/unique_tx.go | 21 +++++--- vms/components/core/block.go | 24 ++++++--- vms/components/missing/block.go | 4 +- vms/components/missing/block_test.go | 21 ++------ vms/platformvm/atomic_block.go | 10 +++- vms/platformvm/common_blocks.go | 28 +++++++--- vms/platformvm/proposal_block.go | 3 +- vms/rpcchainvm/vm_client.go | 8 +-- vms/spchainvm/live_block.go | 10 ++-- vms/spdagvm/unique_tx.go | 16 +++--- 24 files changed, 259 insertions(+), 149 deletions(-) diff --git a/snow/choices/decision.go b/snow/choices/decision.go index a99fd810ccc..8af05d7f263 100644 --- a/snow/choices/decision.go +++ b/snow/choices/decision.go @@ -22,12 +22,12 @@ type Decidable interface { // Accept this element. // // This element will be accepted by every correct node in the network. - Accept() + Accept() error // Reject this element. // // This element will not be accepted by any correct node in the network. - Reject() + Reject() error // Status returns this element's current status. // diff --git a/snow/consensus/avalanche/consensus.go b/snow/consensus/avalanche/consensus.go index 07cf5e48310..2237c68393c 100644 --- a/snow/consensus/avalanche/consensus.go +++ b/snow/consensus/avalanche/consensus.go @@ -34,8 +34,9 @@ type Consensus interface { IsVirtuous(snowstorm.Tx) bool // Adds a new decision. Assumes the dependencies have already been added. - // Assumes that mutations don't conflict with themselves. - Add(Vertex) + // Assumes that mutations don't conflict with themselves. Returns if a + // critical error has occurred. + Add(Vertex) error // VertexIssued returns true iff Vertex has been added VertexIssued(Vertex) bool @@ -54,8 +55,9 @@ type Consensus interface { Preferences() ids.Set // RecordPoll collects the results of a network poll. If a result has not - // been added, the result is dropped. - RecordPoll(ids.UniqueBag) + // been added, the result is dropped. Returns if a critical error has + // occurred. + RecordPoll(ids.UniqueBag) error // Quiesce returns true iff all vertices that have been added but not been accepted or rejected are rogue. // Note, it is possible that after returning quiesce, a new decision may be added such diff --git a/snow/consensus/avalanche/topological.go b/snow/consensus/avalanche/topological.go index 8bee5c397ec..3fa75526a1e 100644 --- a/snow/consensus/avalanche/topological.go +++ b/snow/consensus/avalanche/topological.go @@ -74,7 +74,7 @@ func (ta *Topological) Initialize(ctx *snow.Context, params Parameters, frontier for _, vtx := range frontier { ta.frontier[vtx.ID().Key()] = vtx } - ta.updateFrontiers() + ctx.Log.AssertNoError(ta.updateFrontiers()) } // Parameters implements the Avalanche interface @@ -84,15 +84,15 @@ func (ta *Topological) Parameters() Parameters { return ta.params } func (ta *Topological) IsVirtuous(tx snowstorm.Tx) bool { return ta.cg.IsVirtuous(tx) } // Add implements the Avalanche interface -func (ta *Topological) Add(vtx Vertex) { +func (ta *Topological) Add(vtx Vertex) error { ta.ctx.Log.AssertTrue(vtx != nil, "Attempting to insert nil vertex") vtxID := vtx.ID() key := vtxID.Key() if vtx.Status().Decided() { - return // Already decided this vertex + return nil // Already decided this vertex } else if _, exists := ta.nodes[key]; exists { - return // Already inserted this vertex + return nil // Already inserted this vertex } ta.ctx.ConsensusDispatcher.Issue(ta.ctx.ChainID, vtxID, vtx.Bytes()) @@ -100,14 +100,16 @@ func (ta *Topological) Add(vtx Vertex) { for _, tx := range vtx.Txs() { if !tx.Status().Decided() { // Add the consumers to the conflict graph. - ta.cg.Add(tx) + if err := ta.cg.Add(tx); err != nil { + return err + } } } ta.nodes[key] = vtx // Add this vertex to the set of nodes ta.metrics.Issued(vtxID) - ta.update(vtx) // Update the vertex and it's ancestry + return ta.update(vtx) // Update the vertex and it's ancestry } // VertexIssued implements the Avalanche interface @@ -132,7 +134,7 @@ func (ta *Topological) Virtuous() ids.Set { return ta.virtuous } func (ta *Topological) Preferences() ids.Set { return ta.preferred } // RecordPoll implements the Avalanche interface -func (ta *Topological) RecordPoll(responses ids.UniqueBag) { +func (ta *Topological) RecordPoll(responses ids.UniqueBag) error { // Set up the topological sort: O(|Live Set|) kahns, leaves := ta.calculateInDegree(responses) // Collect the votes for each transaction: O(|Live Set|) @@ -141,7 +143,7 @@ func (ta *Topological) RecordPoll(responses ids.UniqueBag) { ta.ctx.Log.Verbo("Updating consumer confidences based on:\n%s", &votes) ta.cg.RecordPoll(votes) // Update the dag: O(|Live Set|) - ta.updateFrontiers() + return ta.updateFrontiers() } // Quiesce implements the Avalanche interface @@ -275,11 +277,11 @@ func (ta *Topological) pushVotes( // If I'm preferred, remove all my ancestors from the preferred frontier, add // myself to the preferred frontier // If all my parents are accepted and I'm acceptable, accept myself -func (ta *Topological) update(vtx Vertex) { +func (ta *Topological) update(vtx Vertex) error { vtxID := vtx.ID() vtxKey := vtxID.Key() if _, cached := ta.preferenceCache[vtxKey]; cached { - return // This vertex has already been updated + return nil // This vertex has already been updated } switch vtx.Status() { @@ -291,12 +293,12 @@ func (ta *Topological) update(vtx Vertex) { ta.preferenceCache[vtxKey] = true ta.virtuousCache[vtxKey] = true - return + return nil case choices.Rejected: // I'm rejected ta.preferenceCache[vtxKey] = false ta.virtuousCache[vtxKey] = false - return + return nil } acceptable := true // If the batch is accepted, this vertex is acceptable @@ -327,7 +329,9 @@ func (ta *Topological) update(vtx Vertex) { deps := vtx.Parents() // Update all of my dependencies for _, dep := range deps { - ta.update(dep) + if err := ta.update(dep); err != nil { + return err + } depID := dep.ID() key := depID.Key() @@ -338,13 +342,17 @@ func (ta *Topological) update(vtx Vertex) { // Check my parent statuses for _, dep := range deps { if status := dep.Status(); status == choices.Rejected { - vtx.Reject() // My parent is rejected, so I should be rejected + // My parent is rejected, so I should be rejected + if err := vtx.Reject(); err != nil { + return err + } + ta.ctx.ConsensusDispatcher.Reject(ta.ctx.ChainID, vtxID, vtx.Bytes()) delete(ta.nodes, vtxKey) ta.metrics.Rejected(vtxID) ta.preferenceCache[vtxKey] = false ta.virtuousCache[vtxKey] = false - return + return nil } else if status != choices.Accepted { acceptable = false // My parent isn't accepted, so I can't be } @@ -389,21 +397,26 @@ func (ta *Topological) update(vtx Vertex) { switch { case acceptable: // I'm acceptable, why not accept? + if err := vtx.Accept(); err != nil { + return err + } ta.ctx.ConsensusDispatcher.Accept(ta.ctx.ChainID, vtxID, vtx.Bytes()) - vtx.Accept() delete(ta.nodes, vtxKey) ta.metrics.Accepted(vtxID) case rejectable: // I'm rejectable, why not reject? - vtx.Reject() + if err := vtx.Reject(); err != nil { + return err + } ta.ctx.ConsensusDispatcher.Reject(ta.ctx.ChainID, vtxID, vtx.Bytes()) delete(ta.nodes, vtxKey) ta.metrics.Rejected(vtxID) } + return nil } // Update the frontier sets -func (ta *Topological) updateFrontiers() { +func (ta *Topological) updateFrontiers() error { vts := ta.frontier ta.preferred.Clear() @@ -417,6 +430,9 @@ func (ta *Topological) updateFrontiers() { for _, vtx := range vts { // Update all the vertices that were in my previous frontier - ta.update(vtx) + if err := ta.update(vtx); err != nil { + return err + } } + return nil } diff --git a/snow/consensus/avalanche/vertex_test.go b/snow/consensus/avalanche/vertex_test.go index 14b02866bf4..6becebaa565 100644 --- a/snow/consensus/avalanche/vertex_test.go +++ b/snow/consensus/avalanche/vertex_test.go @@ -28,8 +28,8 @@ func (v *Vtx) Parents() []Vertex { return v.dependencies } func (v *Vtx) Txs() []snowstorm.Tx { return v.txs } func (v *Vtx) Status() choices.Status { return v.status } func (v *Vtx) Live() {} -func (v *Vtx) Accept() { v.status = choices.Accepted } -func (v *Vtx) Reject() { v.status = choices.Rejected } +func (v *Vtx) Accept() error { v.status = choices.Accepted; return nil } +func (v *Vtx) Reject() error { v.status = choices.Rejected; return nil } func (v *Vtx) Bytes() []byte { return v.bytes } type sortVts []*Vtx diff --git a/snow/consensus/snowman/block_test.go b/snow/consensus/snowman/block_test.go index 8b3b06f268d..e03e6c25b9e 100644 --- a/snow/consensus/snowman/block_test.go +++ b/snow/consensus/snowman/block_test.go @@ -4,6 +4,7 @@ package snowman import ( + "errors" "sort" "github.com/ava-labs/gecko/ids" @@ -21,17 +22,19 @@ type TestBlock struct { func (b *TestBlock) Parent() Block { return b.parent } func (b *TestBlock) ID() ids.ID { return b.id } func (b *TestBlock) Status() choices.Status { return b.status } -func (b *TestBlock) Accept() { +func (b *TestBlock) Accept() error { if b.status.Decided() && b.status != choices.Accepted { - panic("Dis-agreement") + return errors.New("Dis-agreement") } b.status = choices.Accepted + return nil } -func (b *TestBlock) Reject() { +func (b *TestBlock) Reject() error { if b.status.Decided() && b.status != choices.Rejected { - panic("Dis-agreement") + return errors.New("Dis-agreement") } b.status = choices.Rejected + return nil } func (b *TestBlock) Verify() error { return nil } func (b *TestBlock) Bytes() []byte { return b.bytes } diff --git a/snow/consensus/snowman/consensus.go b/snow/consensus/snowman/consensus.go index fe4db082186..efece649fa8 100644 --- a/snow/consensus/snowman/consensus.go +++ b/snow/consensus/snowman/consensus.go @@ -19,7 +19,8 @@ type Consensus interface { Parameters() snowball.Parameters // Adds a new decision. Assumes the dependency has already been added. - Add(Block) + // Returns if a critical error has occurred. + Add(Block) error // Issued returns true if the block has been issued into consensus Issued(Block) bool @@ -29,8 +30,8 @@ type Consensus interface { Preference() ids.ID // RecordPoll collects the results of a network poll. Assumes all decisions - // have been previously added. - RecordPoll(ids.Bag) + // have been previously added. Returns if a critical error has occurred. + RecordPoll(ids.Bag) error // Finalized returns true if all decisions that have been added have been // finalized. Note, it is possible that after returning finalized, a new diff --git a/snow/consensus/snowman/topological.go b/snow/consensus/snowman/topological.go index e852030dd66..6f987519712 100644 --- a/snow/consensus/snowman/topological.go +++ b/snow/consensus/snowman/topological.go @@ -77,7 +77,7 @@ func (ts *Topological) Initialize(ctx *snow.Context, params snowball.Parameters, func (ts *Topological) Parameters() snowball.Parameters { return ts.params } // Add implements the Snowman interface -func (ts *Topological) Add(blk Block) { +func (ts *Topological) Add(blk Block) error { parent := blk.Parent() parentID := parent.ID() parentKey := parentID.Key() @@ -95,13 +95,15 @@ func (ts *Topological) Add(blk Block) { // If the ancestor is missing, this means the ancestor must have already // been pruned. Therefore, the dependent should be transitively // rejected. - blk.Reject() + if err := blk.Reject(); err != nil { + return err + } // Notify anyone listening that this block was rejected. ts.ctx.DecisionDispatcher.Reject(ts.ctx.ChainID, blkID, blkBytes) ts.ctx.ConsensusDispatcher.Reject(ts.ctx.ChainID, blkID, blkBytes) ts.metrics.Rejected(blkID) - return + return nil } // add the block as a child of its parent, and add the block to the tree @@ -115,6 +117,7 @@ func (ts *Topological) Add(blk Block) { if ts.tail.Equals(parentID) { ts.tail = blkID } + return nil } // Issued implements the Snowman interface @@ -154,7 +157,7 @@ func (ts *Topological) Preference() ids.ID { return ts.tail } // The complexity of this function is: // - Runtime = 3 * |live set| + |votes| // - Space = 2 * |live set| + |votes| -func (ts *Topological) RecordPoll(votes ids.Bag) { +func (ts *Topological) RecordPoll(votes ids.Bag) error { // Runtime = |live set| + |votes| ; Space = |live set| + |votes| kahnGraph, leaves := ts.calculateInDegree(votes) @@ -162,10 +165,14 @@ func (ts *Topological) RecordPoll(votes ids.Bag) { voteStack := ts.pushVotes(kahnGraph, leaves) // Runtime = |live set| ; Space = Constant - preferred := ts.vote(voteStack) + preferred, err := ts.vote(voteStack) + if err != nil { + return err + } // Runtime = |live set| ; Space = Constant ts.tail = ts.getPreferredDecendent(preferred) + return nil } // Finalized implements the Snowman interface @@ -292,7 +299,7 @@ func (ts *Topological) pushVotes( } // apply votes to the branch that received an Alpha threshold -func (ts *Topological) vote(voteStack []votes) ids.ID { +func (ts *Topological) vote(voteStack []votes) (ids.ID, error) { // If the voteStack is empty, then the full tree should falter. This won't // change the preferred branch. if len(voteStack) == 0 { @@ -301,7 +308,7 @@ func (ts *Topological) vote(voteStack []votes) ids.ID { headKey := ts.head.Key() headBlock := ts.blocks[headKey] headBlock.shouldFalter = true - return ts.tail + return ts.tail, nil } // keep track of the new preferred block @@ -341,7 +348,9 @@ func (ts *Topological) vote(voteStack []votes) ids.ID { // Only accept when you are finalized and the head. if parentBlock.sb.Finalized() && ts.head.Equals(vote.parentID) { - ts.accept(parentBlock) + if err := ts.accept(parentBlock); err != nil { + return ids.ID{}, err + } // by accepting the child of parentBlock, the last accepted block is // no longer voteParentID, but its child. So, voteParentID can be @@ -393,7 +402,7 @@ func (ts *Topological) vote(voteStack []votes) ids.ID { } } } - return newPreferred + return newPreferred, nil } // Get the preferred decendent of the provided block ID @@ -409,7 +418,7 @@ func (ts *Topological) getPreferredDecendent(blkID ids.ID) ids.ID { // accept the preferred child of the provided snowman block. By accepting the // preferred child, all other children will be rejected. When these children are // rejected, all their descendants will be rejected. -func (ts *Topological) accept(n *snowmanBlock) { +func (ts *Topological) accept(n *snowmanBlock) error { // We are finalizing the block's child, so we need to get the preference pref := n.sb.Preference() @@ -417,7 +426,9 @@ func (ts *Topological) accept(n *snowmanBlock) { // Get the child and accept it child := n.children[pref.Key()] - child.Accept() + if err := child.Accept(); err != nil { + return err + } // Notify anyone listening that this block was accepted. bytes := child.Bytes() @@ -439,7 +450,9 @@ func (ts *Topological) accept(n *snowmanBlock) { continue } - child.Reject() + if err := child.Reject(); err != nil { + return err + } // Notify anyone listening that this block was rejected. bytes := child.Bytes() @@ -452,11 +465,11 @@ func (ts *Topological) accept(n *snowmanBlock) { } // reject all the descendants of the blocks we just rejected - ts.rejectTransitively(rejects) + return ts.rejectTransitively(rejects) } // Takes in a list of rejected ids and rejects all descendants of these IDs -func (ts *Topological) rejectTransitively(rejected []ids.ID) { +func (ts *Topological) rejectTransitively(rejected []ids.ID) error { // the rejected array is treated as a queue, with the next element at index // 0 and the last element at the end of the slice. for len(rejected) > 0 { @@ -471,7 +484,9 @@ func (ts *Topological) rejectTransitively(rejected []ids.ID) { delete(ts.blocks, rejectedKey) for childIDKey, child := range rejectedNode.children { - child.Reject() + if err := child.Reject(); err != nil { + return err + } // Notify anyone listening that this block was rejected. childID := ids.NewID(childIDKey) @@ -484,4 +499,5 @@ func (ts *Topological) rejectTransitively(rejected []ids.ID) { rejected = append(rejected, childID) } } + return nil } diff --git a/snow/consensus/snowstorm/consensus.go b/snow/consensus/snowstorm/consensus.go index 0e05a125831..3a77804f1d5 100644 --- a/snow/consensus/snowstorm/consensus.go +++ b/snow/consensus/snowstorm/consensus.go @@ -28,8 +28,9 @@ type Consensus interface { // That is, no transaction has been added that conflicts with IsVirtuous(Tx) bool - // Adds a new transaction to vote on - Add(Tx) + // Adds a new transaction to vote on. Returns if a critical error has + // occurred. + Add(Tx) error // Returns true iff transaction has been added Issued(Tx) bool @@ -45,8 +46,8 @@ type Consensus interface { Conflicts(Tx) ids.Set // Collects the results of a network poll. Assumes all transactions - // have been previously added - RecordPoll(ids.Bag) + // have been previously added. Returns if a critical error has occurred. + RecordPoll(ids.Bag) error // Returns true iff all remaining transactions are rogue. Note, it is // possible that after returning quiesce, a new decision may be added such diff --git a/snow/consensus/snowstorm/directed.go b/snow/consensus/snowstorm/directed.go index 570bfce7ea1..116f3b7c625 100644 --- a/snow/consensus/snowstorm/directed.go +++ b/snow/consensus/snowstorm/directed.go @@ -14,6 +14,7 @@ import ( "github.com/ava-labs/gecko/snow/consensus/snowball" "github.com/ava-labs/gecko/snow/events" "github.com/ava-labs/gecko/utils/formatting" + "github.com/ava-labs/gecko/utils/wrappers" ) // DirectedFactory implements Factory by returning a directed struct @@ -54,6 +55,8 @@ type Directed struct { // Number of times RecordPoll has been called currentVote int + + errs wrappers.Errs } type flatNode struct { @@ -118,9 +121,9 @@ func (dg *Directed) Conflicts(tx Tx) ids.Set { } // Add implements the Consensus interface -func (dg *Directed) Add(tx Tx) { +func (dg *Directed) Add(tx Tx) error { if dg.Issued(tx) { - return // Already inserted + return nil // Already inserted } txID := tx.ID() @@ -130,11 +133,13 @@ func (dg *Directed) Add(tx Tx) { inputs := tx.InputIDs() // If there are no inputs, Tx is vacuously accepted if inputs.Len() == 0 { - tx.Accept() + if err := tx.Accept(); err != nil { + return err + } dg.ctx.DecisionDispatcher.Accept(dg.ctx.ChainID, txID, bytes) dg.metrics.Issued(txID) dg.metrics.Accepted(txID) - return + return nil } fn := &flatNode{tx: tx} @@ -195,6 +200,7 @@ func (dg *Directed) Add(tx Tx) { } } dg.pendingReject.Register(toReject) + return dg.errs.Err } // Issued implements the Consensus interface @@ -213,7 +219,7 @@ func (dg *Directed) Virtuous() ids.Set { return dg.virtuous } func (dg *Directed) Preferences() ids.Set { return dg.preferences } // RecordPoll implements the Consensus interface -func (dg *Directed) RecordPoll(votes ids.Bag) { +func (dg *Directed) RecordPoll(votes ids.Bag) error { dg.currentVote++ votes.SetThreshold(dg.params.Alpha) @@ -231,7 +237,8 @@ func (dg *Directed) RecordPoll(votes ids.Bag) { } fn.lastVote = dg.currentVote - dg.ctx.Log.Verbo("Increasing (bias, confidence) of %s from (%d, %d) to (%d, %d)", toInc, fn.bias, fn.confidence, fn.bias+1, fn.confidence+1) + dg.ctx.Log.Verbo("Increasing (bias, confidence) of %s from (%d, %d) to (%d, %d)", + toInc, fn.bias, fn.confidence, fn.bias+1, fn.confidence+1) fn.bias++ fn.confidence++ @@ -240,17 +247,22 @@ func (dg *Directed) RecordPoll(votes ids.Bag) { ((!fn.rogue && fn.confidence >= dg.params.BetaVirtuous) || fn.confidence >= dg.params.BetaRogue) { dg.deferAcceptance(fn) + if dg.errs.Errored() { + return dg.errs.Err + } } if !fn.accepted { dg.redirectEdges(fn) } } + return dg.errs.Err } // Quiesce implements the Consensus interface func (dg *Directed) Quiesce() bool { numVirtuous := dg.virtuousVoting.Len() - dg.ctx.Log.Verbo("Conflict graph has %d voting virtuous transactions and %d transactions", numVirtuous, len(dg.nodes)) + dg.ctx.Log.Verbo("Conflict graph has %d voting virtuous transactions and %d transactions", + numVirtuous, len(dg.nodes)) return numVirtuous == 0 } @@ -311,7 +323,7 @@ func (dg *Directed) deferAcceptance(fn *flatNode) { dg.pendingAccept.Register(toAccept) } -func (dg *Directed) reject(ids ...ids.ID) { +func (dg *Directed) reject(ids ...ids.ID) error { for _, conflict := range ids { conflictKey := conflict.Key() conf := dg.nodes[conflictKey] @@ -324,13 +336,16 @@ func (dg *Directed) reject(ids ...ids.ID) { dg.removeConflict(conflict, conf.outs.List()...) // Mark it as rejected - conf.tx.Reject() + if err := conf.tx.Reject(); err != nil { + return err + } dg.ctx.DecisionDispatcher.Reject(dg.ctx.ChainID, conf.tx.ID(), conf.tx.Bytes()) dg.metrics.Rejected(conflict) dg.pendingAccept.Abandon(conflict) dg.pendingReject.Fulfill(conflict) } + return nil } func (dg *Directed) redirectEdges(fn *flatNode) { @@ -396,7 +411,7 @@ func (a *directedAccepter) Abandon(id ids.ID) { a.rejected = true } func (a *directedAccepter) Update() { // If I was rejected or I am still waiting on dependencies to finish do nothing. - if a.rejected || a.deps.Len() != 0 { + if a.rejected || a.deps.Len() != 0 || a.dg.errs.Errored() { return } @@ -410,12 +425,22 @@ func (a *directedAccepter) Update() { a.dg.preferences.Remove(id) // Reject the conflicts - a.dg.reject(a.fn.ins.List()...) - a.dg.reject(a.fn.outs.List()...) // Should normally be empty + if err := a.dg.reject(a.fn.ins.List()...); err != nil { + a.dg.errs.Add(err) + return + } + // Should normally be empty + if err := a.dg.reject(a.fn.outs.List()...); err != nil { + a.dg.errs.Add(err) + return + } // Mark it as accepted + if err := a.fn.tx.Accept(); err != nil { + a.dg.errs.Add(err) + return + } a.fn.accepted = true - a.fn.tx.Accept() a.dg.ctx.DecisionDispatcher.Accept(a.dg.ctx.ChainID, id, a.fn.tx.Bytes()) a.dg.metrics.Accepted(id) @@ -434,11 +459,11 @@ type directedRejector struct { func (r *directedRejector) Dependencies() ids.Set { return r.deps } func (r *directedRejector) Fulfill(id ids.ID) { - if r.rejected { + if r.rejected || r.dg.errs.Errored() { return } r.rejected = true - r.dg.reject(r.fn.tx.ID()) + r.dg.errs.Add(r.dg.reject(r.fn.tx.ID())) } func (*directedRejector) Abandon(id ids.ID) {} diff --git a/snow/consensus/snowstorm/input.go b/snow/consensus/snowstorm/input.go index 76465623044..54d972d7a31 100644 --- a/snow/consensus/snowstorm/input.go +++ b/snow/consensus/snowstorm/input.go @@ -14,6 +14,7 @@ import ( "github.com/ava-labs/gecko/snow/consensus/snowball" "github.com/ava-labs/gecko/snow/events" "github.com/ava-labs/gecko/utils/formatting" + "github.com/ava-labs/gecko/utils/wrappers" ) // InputFactory implements Factory by returning an input struct @@ -43,6 +44,8 @@ type Input struct { // Number of times RecordPoll has been called currentVote int + + errs wrappers.Errs } type txNode struct { @@ -92,9 +95,9 @@ func (ig *Input) IsVirtuous(tx Tx) bool { } // Add implements the ConflictGraph interface -func (ig *Input) Add(tx Tx) { +func (ig *Input) Add(tx Tx) error { if ig.Issued(tx) { - return // Already inserted + return nil // Already inserted } txID := tx.ID() @@ -104,11 +107,13 @@ func (ig *Input) Add(tx Tx) { inputs := tx.InputIDs() // If there are no inputs, they are vacuously accepted if inputs.Len() == 0 { - tx.Accept() + if err := tx.Accept(); err != nil { + return err + } ig.ctx.DecisionDispatcher.Accept(ig.ctx.ChainID, txID, bytes) ig.metrics.Issued(txID) ig.metrics.Accepted(txID) - return + return nil } cn := txNode{tx: tx} @@ -155,6 +160,7 @@ func (ig *Input) Add(tx Tx) { } } ig.pendingReject.Register(toReject) + return ig.errs.Err } // Issued implements the ConflictGraph interface @@ -187,7 +193,7 @@ func (ig *Input) Conflicts(tx Tx) ids.Set { } // RecordPoll implements the ConflictGraph interface -func (ig *Input) RecordPoll(votes ids.Bag) { +func (ig *Input) RecordPoll(votes ids.Bag) error { ig.currentVote++ votes.SetThreshold(ig.params.Alpha) @@ -261,11 +267,15 @@ func (ig *Input) RecordPoll(votes ids.Bag) { if (!rogue && confidence >= ig.params.BetaVirtuous) || confidence >= ig.params.BetaRogue { ig.deferAcceptance(tx) + if ig.errs.Errored() { + return ig.errs.Err + } continue } ig.txs[incKey] = tx } + return ig.errs.Err } func (ig *Input) deferAcceptance(tn txNode) { @@ -285,7 +295,7 @@ func (ig *Input) deferAcceptance(tn txNode) { } // reject all the ids and remove them from their conflict sets -func (ig *Input) reject(ids ...ids.ID) { +func (ig *Input) reject(ids ...ids.ID) error { for _, conflict := range ids { conflictKey := conflict.Key() cn := ig.txs[conflictKey] @@ -296,12 +306,15 @@ func (ig *Input) reject(ids ...ids.ID) { ig.removeConflict(conflict, cn.tx.InputIDs().List()...) // Mark it as rejected - cn.tx.Reject() + if err := cn.tx.Reject(); err != nil { + return err + } ig.ctx.DecisionDispatcher.Reject(ig.ctx.ChainID, cn.tx.ID(), cn.tx.Bytes()) ig.metrics.Rejected(conflict) ig.pendingAccept.Abandon(conflict) ig.pendingReject.Fulfill(conflict) } + return nil } // Remove id from all of its conflict sets @@ -458,7 +471,7 @@ func (a *inputAccepter) Fulfill(id ids.ID) { func (a *inputAccepter) Abandon(id ids.ID) { a.rejected = true } func (a *inputAccepter) Update() { - if a.rejected || a.deps.Len() != 0 { + if a.rejected || a.deps.Len() != 0 || a.ig.errs.Errored() { return } @@ -480,10 +493,16 @@ func (a *inputAccepter) Update() { conflicts.Union(inputNode.conflicts) } } - a.ig.reject(conflicts.List()...) + if err := a.ig.reject(conflicts.List()...); err != nil { + a.ig.errs.Add(err) + return + } // Mark it as accepted - a.tn.tx.Accept() + if err := a.tn.tx.Accept(); err != nil { + a.ig.errs.Add(err) + return + } a.ig.ctx.DecisionDispatcher.Accept(a.ig.ctx.ChainID, id, a.tn.tx.Bytes()) a.ig.metrics.Accepted(id) @@ -502,11 +521,11 @@ type inputRejector struct { func (r *inputRejector) Dependencies() ids.Set { return r.deps } func (r *inputRejector) Fulfill(id ids.ID) { - if r.rejected { + if r.rejected || r.ig.errs.Errored() { return } r.rejected = true - r.ig.reject(r.tn.tx.ID()) + r.ig.errs.Add(r.ig.reject(r.tn.tx.ID())) } func (*inputRejector) Abandon(id ids.ID) {} diff --git a/snow/consensus/snowstorm/test_tx.go b/snow/consensus/snowstorm/test_tx.go index 12c2f732ccb..18f1465a012 100644 --- a/snow/consensus/snowstorm/test_tx.go +++ b/snow/consensus/snowstorm/test_tx.go @@ -31,10 +31,10 @@ func (tx *TestTx) InputIDs() ids.Set { return tx.Ins } func (tx *TestTx) Status() choices.Status { return tx.Stat } // Accept implements the Consumer interface -func (tx *TestTx) Accept() { tx.Stat = choices.Accepted } +func (tx *TestTx) Accept() error { tx.Stat = choices.Accepted; return nil } // Reject implements the Consumer interface -func (tx *TestTx) Reject() { tx.Stat = choices.Rejected } +func (tx *TestTx) Reject() error { tx.Stat = choices.Rejected; return nil } // Reset sets the status to pending func (tx *TestTx) Reset() { tx.Stat = choices.Processing } diff --git a/snow/engine/avalanche/engine_test.go b/snow/engine/avalanche/engine_test.go index 1b1c013c3d9..ee16c8aab2a 100644 --- a/snow/engine/avalanche/engine_test.go +++ b/snow/engine/avalanche/engine_test.go @@ -38,8 +38,8 @@ func (v *Vtx) DependencyIDs() []ids.ID { return nil } func (v *Vtx) Parents() []avalanche.Vertex { return v.parents } func (v *Vtx) Txs() []snowstorm.Tx { return v.txs } func (v *Vtx) Status() choices.Status { return v.status } -func (v *Vtx) Accept() { v.status = choices.Accepted } -func (v *Vtx) Reject() { v.status = choices.Rejected } +func (v *Vtx) Accept() error { v.status = choices.Accepted; return nil } +func (v *Vtx) Reject() error { v.status = choices.Rejected; return nil } func (v *Vtx) Bytes() []byte { return v.bytes } type sortVts []*Vtx diff --git a/snow/engine/avalanche/state/unique_vertex.go b/snow/engine/avalanche/state/unique_vertex.go index fe88c63c816..917a77f7646 100644 --- a/snow/engine/avalanche/state/unique_vertex.go +++ b/snow/engine/avalanche/state/unique_vertex.go @@ -76,7 +76,7 @@ func (vtx *uniqueVertex) setStatus(status choices.Status) { func (vtx *uniqueVertex) ID() ids.ID { return vtx.vtxID } -func (vtx *uniqueVertex) Accept() { +func (vtx *uniqueVertex) Accept() error { vtx.setStatus(choices.Accepted) vtx.serializer.edge.Add(vtx.vtxID) @@ -90,17 +90,17 @@ func (vtx *uniqueVertex) Accept() { // parents to be garbage collected vtx.v.parents = nil - vtx.serializer.db.Commit() + return vtx.serializer.db.Commit() } -func (vtx *uniqueVertex) Reject() { +func (vtx *uniqueVertex) Reject() error { vtx.setStatus(choices.Rejected) // Should never traverse into parents of a decided vertex. Allows for the // parents to be garbage collected vtx.v.parents = nil - vtx.serializer.db.Commit() + return vtx.serializer.db.Commit() } func (vtx *uniqueVertex) Status() choices.Status { vtx.refresh(); return vtx.v.status } diff --git a/snow/engine/snowman/engine_test.go b/snow/engine/snowman/engine_test.go index bc4ed593a78..d034a3033f1 100644 --- a/snow/engine/snowman/engine_test.go +++ b/snow/engine/snowman/engine_test.go @@ -34,8 +34,8 @@ type Blk struct { func (b *Blk) ID() ids.ID { return b.id } func (b *Blk) Parent() snowman.Block { return b.parent } -func (b *Blk) Accept() { b.status = choices.Accepted } -func (b *Blk) Reject() { b.status = choices.Rejected } +func (b *Blk) Accept() error { b.status = choices.Accepted; return nil } +func (b *Blk) Reject() error { b.status = choices.Rejected; return nil } func (b *Blk) Status() choices.Status { return b.status } func (b *Blk) Verify() error { return b.validity } func (b *Blk) Bytes() []byte { return b.bytes } diff --git a/vms/avm/unique_tx.go b/vms/avm/unique_tx.go index 788ef523465..03d43953eba 100644 --- a/vms/avm/unique_tx.go +++ b/vms/avm/unique_tx.go @@ -99,12 +99,12 @@ func (tx *UniqueTx) setStatus(status choices.Status) error { func (tx *UniqueTx) ID() ids.ID { return tx.txID } // Accept is called when the transaction was finalized as accepted by consensus -func (tx *UniqueTx) Accept() { +func (tx *UniqueTx) Accept() error { defer tx.vm.db.Abort() if err := tx.setStatus(choices.Accepted); err != nil { tx.vm.ctx.Log.Error("Failed to accept tx %s due to %s", tx.txID, err) - return + return err } // Remove spent utxos @@ -116,7 +116,7 @@ func (tx *UniqueTx) Accept() { utxoID := utxo.InputID() if err := tx.vm.state.SpendUTXO(utxoID); err != nil { tx.vm.ctx.Log.Error("Failed to spend utxo %s due to %s", utxoID, err) - return + return err } } @@ -124,7 +124,7 @@ func (tx *UniqueTx) Accept() { for _, utxo := range tx.UTXOs() { if err := tx.vm.state.FundUTXO(utxo); err != nil { tx.vm.ctx.Log.Error("Failed to fund utxo %s due to %s", utxoID, err) - return + return err } } @@ -132,12 +132,12 @@ func (tx *UniqueTx) Accept() { commitBatch, err := tx.vm.db.CommitBatch() if err != nil { tx.vm.ctx.Log.Error("Failed to calculate CommitBatch for %s due to %s", txID, err) - return + return err } if err := tx.ExecuteWithSideEffects(tx.vm, commitBatch); err != nil { tx.vm.ctx.Log.Error("Failed to commit accept %s due to %s", txID, err) - return + return err } tx.vm.ctx.Log.Verbo("Accepted Tx: %s", txID) @@ -149,15 +149,17 @@ func (tx *UniqueTx) Accept() { if tx.onDecide != nil { tx.onDecide(choices.Accepted) } + + return nil } // Reject is called when the transaction was finalized as rejected by consensus -func (tx *UniqueTx) Reject() { +func (tx *UniqueTx) Reject() error { defer tx.vm.db.Abort() if err := tx.setStatus(choices.Rejected); err != nil { tx.vm.ctx.Log.Error("Failed to reject tx %s due to %s", tx.txID, err) - return + return err } txID := tx.ID() @@ -165,6 +167,7 @@ func (tx *UniqueTx) Reject() { if err := tx.vm.db.Commit(); err != nil { tx.vm.ctx.Log.Error("Failed to commit reject %s due to %s", tx.txID, err) + return err } tx.vm.pubsub.Publish("rejected", txID) @@ -174,6 +177,8 @@ func (tx *UniqueTx) Reject() { if tx.onDecide != nil { tx.onDecide(choices.Rejected) } + + return nil } // Status returns the current status of this transaction diff --git a/vms/components/core/block.go b/vms/components/core/block.go index cfd9d36788d..3609477ac1d 100644 --- a/vms/components/core/block.go +++ b/vms/components/core/block.go @@ -53,18 +53,28 @@ func (b *Block) Parent() snowman.Block { // Accept sets this block's status to Accepted and sets lastAccepted to this // block's ID and saves this info to b.vm.DB // Recall that b.vm.DB.Commit() must be called to persist to the DB -func (b *Block) Accept() { - b.SetStatus(choices.Accepted) // Change state of this block - b.VM.State.PutStatus(b.VM.DB, b.ID(), choices.Accepted) // Persist data - b.VM.State.PutLastAccepted(b.VM.DB, b.ID()) - b.VM.LastAcceptedID = b.ID() // Change state of VM +func (b *Block) Accept() error { + b.SetStatus(choices.Accepted) // Change state of this block + + blkID := b.ID() + + // Persist data + if err := b.VM.State.PutStatus(b.VM.DB, blkID, choices.Accepted); err != nil { + return err + } + if err := b.VM.State.PutLastAccepted(b.VM.DB, blkID); err != nil { + return err + } + + b.VM.LastAcceptedID = blkID // Change state of VM + return nil } // Reject sets this block's status to Rejected and saves the status in state // Recall that b.vm.DB.Commit() must be called to persist to the DB -func (b *Block) Reject() { +func (b *Block) Reject() error { b.SetStatus(choices.Rejected) - b.VM.State.PutStatus(b.VM.DB, b.ID(), choices.Rejected) + return b.VM.State.PutStatus(b.VM.DB, b.ID(), choices.Rejected) } // Status returns the status of this block diff --git a/vms/components/missing/block.go b/vms/components/missing/block.go index 3f9b4a22e29..85610cb243d 100644 --- a/vms/components/missing/block.go +++ b/vms/components/missing/block.go @@ -22,10 +22,10 @@ type Block struct{ BlkID ids.ID } func (mb *Block) ID() ids.ID { return mb.BlkID } // Accept ... -func (*Block) Accept() { panic(errMissingBlock) } +func (*Block) Accept() error { return errMissingBlock } // Reject ... -func (*Block) Reject() { panic(errMissingBlock) } +func (*Block) Reject() error { return errMissingBlock } // Status ... func (*Block) Status() choices.Status { return choices.Unknown } diff --git a/vms/components/missing/block_test.go b/vms/components/missing/block_test.go index 9bffa996911..f838bc9a0b0 100644 --- a/vms/components/missing/block_test.go +++ b/vms/components/missing/block_test.go @@ -24,22 +24,9 @@ func TestMissingBlock(t *testing.T) { t.Fatalf("missingBlock.Verify returned nil, expected an error") } else if bytes := mb.Bytes(); bytes != nil { t.Fatalf("missingBlock.Bytes returned %v, expected %v", bytes, nil) + } else if err := mb.Accept(); err == nil { + t.Fatalf("missingBlock.Accept should have returned an error") + } else if err := mb.Reject(); err == nil { + t.Fatalf("missingBlock.Reject should have returned an error") } - - func() { - defer func() { - if r := recover(); r == nil { - t.Fatalf("Should have panicked on accept") - } - }() - mb.Accept() - }() - func() { - defer func() { - if r := recover(); r == nil { - t.Fatalf("Should have panicked on reject") - } - }() - mb.Reject() - }() } diff --git a/vms/platformvm/atomic_block.go b/vms/platformvm/atomic_block.go index 32930943128..914170f7d16 100644 --- a/vms/platformvm/atomic_block.go +++ b/vms/platformvm/atomic_block.go @@ -96,24 +96,29 @@ func (ab *AtomicBlock) Verify() error { } // Accept implements the snowman.Block interface -func (ab *AtomicBlock) Accept() { +func (ab *AtomicBlock) Accept() error { ab.vm.Ctx.Log.Verbo("Accepting block with ID %s", ab.ID()) - ab.CommonBlock.Accept() + if err := ab.CommonBlock.Accept(); err != nil { + return err + } // Update the state of the chain in the database if err := ab.onAcceptDB.Commit(); err != nil { ab.vm.Ctx.Log.Error("unable to commit onAcceptDB") + return err } batch, err := ab.vm.DB.CommitBatch() if err != nil { ab.vm.Ctx.Log.Fatal("unable to commit vm's DB") + return err } defer ab.vm.DB.Abort() if err := ab.Tx.Accept(batch); err != nil { ab.vm.Ctx.Log.Error("unable to atomically commit block") + return err } for _, child := range ab.children { @@ -124,6 +129,7 @@ func (ab *AtomicBlock) Accept() { } ab.free() + return nil } // newAtomicBlock returns a new *AtomicBlock where the block's parent, a diff --git a/vms/platformvm/common_blocks.go b/vms/platformvm/common_blocks.go index cdbf54822f6..fe53b915cf3 100644 --- a/vms/platformvm/common_blocks.go +++ b/vms/platformvm/common_blocks.go @@ -129,10 +129,10 @@ type CommonBlock struct { } // Reject implements the snowman.Block interface -func (cb *CommonBlock) Reject() { +func (cb *CommonBlock) Reject() error { defer cb.free() // remove this block from memory - cb.Block.Reject() + return cb.Block.Reject() } // free removes this block from memory @@ -213,17 +213,21 @@ type SingleDecisionBlock struct { } // Accept implements the snowman.Block interface -func (sdb *SingleDecisionBlock) Accept() { +func (sdb *SingleDecisionBlock) Accept() error { sdb.VM.Ctx.Log.Verbo("Accepting block with ID %s", sdb.ID()) - sdb.CommonBlock.Accept() + if err := sdb.CommonBlock.Accept(); err != nil { + return err + } // Update the state of the chain in the database if err := sdb.onAcceptDB.Commit(); err != nil { sdb.vm.Ctx.Log.Warn("unable to commit onAcceptDB") + return err } if err := sdb.vm.DB.Commit(); err != nil { sdb.vm.Ctx.Log.Warn("unable to commit vm's DB") + return err } for _, child := range sdb.children { @@ -234,6 +238,7 @@ func (sdb *SingleDecisionBlock) Accept() { } sdb.free() + return nil } // DoubleDecisionBlock contains the accept for a pair of blocks @@ -242,25 +247,31 @@ type DoubleDecisionBlock struct { } // Accept implements the snowman.Block interface -func (ddb *DoubleDecisionBlock) Accept() { +func (ddb *DoubleDecisionBlock) Accept() error { ddb.VM.Ctx.Log.Verbo("Accepting block with ID %s", ddb.ID()) parent, ok := ddb.parentBlock().(*ProposalBlock) if !ok { ddb.vm.Ctx.Log.Error("double decision block should only follow a proposal block") - return + return errInvalidBlockType } - parent.CommonBlock.Accept() + if err := parent.CommonBlock.Accept(); err != nil { + return err + } - ddb.CommonBlock.Accept() + if err := ddb.CommonBlock.Accept(); err != nil { + return err + } // Update the state of the chain in the database if err := ddb.onAcceptDB.Commit(); err != nil { ddb.vm.Ctx.Log.Warn("unable to commit onAcceptDB") + return err } if err := ddb.vm.DB.Commit(); err != nil { ddb.vm.Ctx.Log.Warn("unable to commit vm's DB") + return err } for _, child := range ddb.children { @@ -273,4 +284,5 @@ func (ddb *DoubleDecisionBlock) Accept() { // remove this block and its parent from memory parent.free() ddb.free() + return nil } diff --git a/vms/platformvm/proposal_block.go b/vms/platformvm/proposal_block.go index 7d70286cb99..81f93b1336c 100644 --- a/vms/platformvm/proposal_block.go +++ b/vms/platformvm/proposal_block.go @@ -43,9 +43,10 @@ type ProposalBlock struct { } // Accept implements the snowman.Block interface -func (pb *ProposalBlock) Accept() { +func (pb *ProposalBlock) Accept() error { pb.SetStatus(choices.Accepted) pb.VM.LastAcceptedID = pb.ID() + return nil } // Initialize this block. diff --git a/vms/rpcchainvm/vm_client.go b/vms/rpcchainvm/vm_client.go index 06984323d9b..8ab5229c9b8 100644 --- a/vms/rpcchainvm/vm_client.go +++ b/vms/rpcchainvm/vm_client.go @@ -289,23 +289,23 @@ type BlockClient struct { func (b *BlockClient) ID() ids.ID { return b.id } // Accept ... -func (b *BlockClient) Accept() { +func (b *BlockClient) Accept() error { delete(b.vm.blks, b.id.Key()) b.status = choices.Accepted _, err := b.vm.client.BlockAccept(context.Background(), &vmproto.BlockAcceptRequest{ Id: b.id.Bytes(), }) - b.vm.ctx.Log.AssertNoError(err) + return err } // Reject ... -func (b *BlockClient) Reject() { +func (b *BlockClient) Reject() error { delete(b.vm.blks, b.id.Key()) b.status = choices.Rejected _, err := b.vm.client.BlockReject(context.Background(), &vmproto.BlockRejectRequest{ Id: b.id.Bytes(), }) - b.vm.ctx.Log.AssertNoError(err) + return err } // Status ... diff --git a/vms/spchainvm/live_block.go b/vms/spchainvm/live_block.go index b0f04579ca8..390fa9768eb 100644 --- a/vms/spchainvm/live_block.go +++ b/vms/spchainvm/live_block.go @@ -46,7 +46,7 @@ type LiveBlock struct { func (lb *LiveBlock) ID() ids.ID { return lb.block.id } // Accept is called when this block is finalized as accepted by consensus -func (lb *LiveBlock) Accept() { +func (lb *LiveBlock) Accept() error { bID := lb.ID() lb.vm.ctx.Log.Debug("Accepted block %s", bID) @@ -55,7 +55,7 @@ func (lb *LiveBlock) Accept() { if err := lb.db.Commit(); err != nil { lb.vm.ctx.Log.Debug("Failed to accept block %s due to %s", bID, err) - return + return err } for _, child := range lb.children { @@ -74,15 +74,16 @@ func (lb *LiveBlock) Accept() { if lb.vm.onAccept != nil { lb.vm.onAccept(bID) } + return nil } // Reject is called when this block is finalized as rejected by consensus -func (lb *LiveBlock) Reject() { +func (lb *LiveBlock) Reject() error { lb.vm.ctx.Log.Debug("Rejected block %s", lb.ID()) if err := lb.vm.state.SetStatus(lb.vm.baseDB, lb.ID(), choices.Rejected); err != nil { lb.vm.ctx.Log.Debug("Failed to reject block %s due to %s", lb.ID(), err) - return + return err } lb.status = choices.Rejected @@ -96,6 +97,7 @@ func (lb *LiveBlock) Reject() { tx.onDecide(choices.Rejected) } } + return nil } // Status returns the current status of this block diff --git a/vms/spdagvm/unique_tx.go b/vms/spdagvm/unique_tx.go index cb64d3a1673..64fb6872798 100644 --- a/vms/spdagvm/unique_tx.go +++ b/vms/spdagvm/unique_tx.go @@ -80,17 +80,17 @@ func (tx *UniqueTx) addEvents(finalized func(choices.Status)) { func (tx *UniqueTx) ID() ids.ID { return tx.txID } // Accept is called when the transaction was finalized as accepted by consensus -func (tx *UniqueTx) Accept() { +func (tx *UniqueTx) Accept() error { if err := tx.setStatus(choices.Accepted); err != nil { tx.vm.ctx.Log.Error("Failed to accept tx %s due to %s", tx.txID, err) - return + return err } // Remove spent UTXOs for _, utxoID := range tx.InputIDs().List() { if err := tx.vm.state.SpendUTXO(utxoID); err != nil { tx.vm.ctx.Log.Error("Failed to spend utxo %s due to %s", utxoID, err) - return + return err } } @@ -98,7 +98,7 @@ func (tx *UniqueTx) Accept() { for _, utxo := range tx.utxos() { if err := tx.vm.state.FundUTXO(utxo); err != nil { tx.vm.ctx.Log.Error("Failed to fund utxo %s due to %s", utxoID, err) - return + return err } } @@ -110,16 +110,18 @@ func (tx *UniqueTx) Accept() { if err := tx.vm.db.Commit(); err != nil { tx.vm.ctx.Log.Error("Failed to commit accept %s due to %s", tx.txID, err) + return err } tx.t.deps = nil // Needed to prevent a memory leak + return nil } // Reject is called when the transaction was finalized as rejected by consensus -func (tx *UniqueTx) Reject() { +func (tx *UniqueTx) Reject() error { if err := tx.setStatus(choices.Rejected); err != nil { tx.vm.ctx.Log.Error("Failed to reject tx %s due to %s", tx.txID, err) - return + return err } tx.vm.ctx.Log.Debug("Rejecting Tx: %s", tx.ID()) @@ -132,9 +134,11 @@ func (tx *UniqueTx) Reject() { if err := tx.vm.db.Commit(); err != nil { tx.vm.ctx.Log.Error("Failed to commit reject %s due to %s", tx.txID, err) + return err } tx.t.deps = nil // Needed to prevent a memory leak + return nil } // Status returns the current status of this transaction From 6d82e63ad8a90441a759db067a3e6f69b229b60b Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Tue, 26 May 2020 15:16:23 -0400 Subject: [PATCH 03/29] Added errors to block jobs --- snow/engine/snowman/block_job.go | 26 +++++++++++++++++++++----- snow/engine/snowman/bootstrapper.go | 1 + 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/snow/engine/snowman/block_job.go b/snow/engine/snowman/block_job.go index ec5f4a3421c..c02b756b65c 100644 --- a/snow/engine/snowman/block_job.go +++ b/snow/engine/snowman/block_job.go @@ -4,15 +4,20 @@ package snowman import ( + "errors" + "fmt" + "github.com/prometheus/client_golang/prometheus" "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/choices" "github.com/ava-labs/gecko/snow/consensus/snowman" "github.com/ava-labs/gecko/snow/engine/common/queue" + "github.com/ava-labs/gecko/utils/logging" ) type parser struct { + log logging.Logger numAccepted, numDropped prometheus.Counter vm ChainVM } @@ -23,6 +28,7 @@ func (p *parser) Parse(blkBytes []byte) (queue.Job, error) { return nil, err } return &blockJob{ + log: p.log, numAccepted: p.numAccepted, numDropped: p.numDropped, blk: blk, @@ -30,6 +36,7 @@ func (p *parser) Parse(blkBytes []byte) (queue.Job, error) { } type blockJob struct { + log logging.Logger numAccepted, numDropped prometheus.Counter blk snowman.Block } @@ -42,18 +49,27 @@ func (b *blockJob) MissingDependencies() ids.Set { } return missing } -func (b *blockJob) Execute() { +func (b *blockJob) Execute() error { if b.MissingDependencies().Len() != 0 { b.numDropped.Inc() - return + return errors.New("attempting to accept a block with missing dependencies") } - switch b.blk.Status() { + status := b.blk.Status() + switch status { case choices.Unknown, choices.Rejected: b.numDropped.Inc() + return fmt.Errorf("attempting to execute block with status %s", status) case choices.Processing: - b.blk.Verify() - b.blk.Accept() + if err := b.blk.Verify(); err != nil { + b.log.Warn("block %s failed verification during bootstrapping due to %s", + b.blk.ID(), err) + } + b.numAccepted.Inc() + if err := b.blk.Accept(); err != nil { + return fmt.Errorf("failed to accept block in bootstrapping: %w", err) + } } + return nil } func (b *blockJob) Bytes() []byte { return b.blk.Bytes() } diff --git a/snow/engine/snowman/bootstrapper.go b/snow/engine/snowman/bootstrapper.go index 7442eead89a..5df0f3a40d5 100644 --- a/snow/engine/snowman/bootstrapper.go +++ b/snow/engine/snowman/bootstrapper.go @@ -43,6 +43,7 @@ func (b *bootstrapper) Initialize(config BootstrapConfig) { b.BootstrapConfig = config b.Blocked.SetParser(&parser{ + log: config.Context.Log, numAccepted: b.numBootstrapped, numDropped: b.numDropped, vm: b.VM, From 855d15ec09411850597f1d6293444f2f1c855779 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Tue, 26 May 2020 16:02:41 -0400 Subject: [PATCH 04/29] Updated log levels for bootstrapping --- snow/engine/avalanche/tx_job.go | 4 +++- snow/engine/snowman/block_job.go | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/snow/engine/avalanche/tx_job.go b/snow/engine/avalanche/tx_job.go index d9fbbe1b487..464eb072e0c 100644 --- a/snow/engine/avalanche/tx_job.go +++ b/snow/engine/avalanche/tx_job.go @@ -65,12 +65,14 @@ func (t *txJob) Execute() error { return fmt.Errorf("attempting to execute transaction with status %s", status) case choices.Processing: if err := t.tx.Verify(); err != nil { - t.log.Warn("transaction %s failed verification during bootstrapping due to %s", + t.log.Debug("transaction %s failed verification during bootstrapping due to %s", t.tx.ID(), err) } t.numAccepted.Inc() if err := t.tx.Accept(); err != nil { + t.log.Error("transaction %s failed to accept during bootstrapping due to %s", + t.tx.ID(), err) return fmt.Errorf("failed to accept transaction in bootstrapping: %w", err) } } diff --git a/snow/engine/snowman/block_job.go b/snow/engine/snowman/block_job.go index c02b756b65c..cdf1eea7b19 100644 --- a/snow/engine/snowman/block_job.go +++ b/snow/engine/snowman/block_job.go @@ -61,12 +61,14 @@ func (b *blockJob) Execute() error { return fmt.Errorf("attempting to execute block with status %s", status) case choices.Processing: if err := b.blk.Verify(); err != nil { - b.log.Warn("block %s failed verification during bootstrapping due to %s", + b.log.Debug("block %s failed verification during bootstrapping due to %s", b.blk.ID(), err) } b.numAccepted.Inc() if err := b.blk.Accept(); err != nil { + b.log.Debug("block %s failed to accept during bootstrapping due to %s", + b.blk.ID(), err) return fmt.Errorf("failed to accept block in bootstrapping: %w", err) } } From 198accdea7004adf7cbaa593e223925cb7a6be36 Mon Sep 17 00:00:00 2001 From: Collin Montag Date: Thu, 28 May 2020 10:36:00 -0400 Subject: [PATCH 05/29] ava state prefixdb --- vms/avm/prefixed_state.go | 17 ++-------- vms/avm/state_test.go | 24 +++++++------- vms/components/ava/prefixed_state.go | 16 ++-------- vms/components/ava/state.go | 47 +++++++++++----------------- 4 files changed, 35 insertions(+), 69 deletions(-) diff --git a/vms/avm/prefixed_state.go b/vms/avm/prefixed_state.go index 8a1898d04e9..862771a6d17 100644 --- a/vms/avm/prefixed_state.go +++ b/vms/avm/prefixed_state.go @@ -80,11 +80,6 @@ func (s *prefixedState) Funds(id ids.ID) ([]ids.ID, error) { return s.state.IDs(uniqueID(id, fundsID, s.funds)) } -// SetFunds saves the mapping from address to utxo IDs to storage. -func (s *prefixedState) SetFunds(id ids.ID, idSlice []ids.ID) error { - return s.state.SetIDs(uniqueID(id, fundsID, s.funds), idSlice) -} - // SpendUTXO consumes the provided utxo. func (s *prefixedState) SpendUTXO(utxoID ids.ID) error { utxo, err := s.UTXO(utxoID) @@ -106,11 +101,7 @@ func (s *prefixedState) SpendUTXO(utxoID ids.ID) error { func (s *prefixedState) removeUTXO(addrs [][]byte, utxoID ids.ID) error { for _, addr := range addrs { addrID := ids.NewID(hashing.ComputeHash256Array(addr)) - utxos := ids.Set{} - funds, _ := s.Funds(addrID) - utxos.Add(funds...) - utxos.Remove(utxoID) - if err := s.SetFunds(addrID, utxos.List()); err != nil { + if err := s.state.RemoveID(addrID, utxoID); err != nil { return err } } @@ -135,11 +126,7 @@ func (s *prefixedState) FundUTXO(utxo *ava.UTXO) error { func (s *prefixedState) addUTXO(addrs [][]byte, utxoID ids.ID) error { for _, addr := range addrs { addrID := ids.NewID(hashing.ComputeHash256Array(addr)) - utxos := ids.Set{} - funds, _ := s.Funds(addrID) - utxos.Add(funds...) - utxos.Add(utxoID) - if err := s.SetFunds(addrID, utxos.List()); err != nil { + if err := s.state.AddID(addrID, utxoID); err != nil { return err } } diff --git a/vms/avm/state_test.go b/vms/avm/state_test.go index e0598cc2e7f..de5810be2dc 100644 --- a/vms/avm/state_test.go +++ b/vms/avm/state_test.go @@ -27,13 +27,15 @@ func TestStateIDs(t *testing.T) { id1 := ids.NewID([32]byte{0xff, 0}) id2 := ids.NewID([32]byte{0xff, 0}) - if _, err := state.IDs(ids.Empty); err == nil { - t.Fatalf("Should have errored when reading ids") + if _, err := state.IDs(ids.Empty); err != nil { + t.Fatal(err) } expected := []ids.ID{id0, id1} - if err := state.SetIDs(ids.Empty, expected); err != nil { - t.Fatal(err) + for _, id := range expected { + if err := state.AddID(ids.Empty, id); err != nil { + t.Fatal(err) + } } result, err := state.IDs(ids.Empty) @@ -53,8 +55,10 @@ func TestStateIDs(t *testing.T) { } expected = []ids.ID{id1, id2} - if err := state.SetIDs(ids.Empty, expected); err != nil { - t.Fatal(err) + for _, id := range expected { + if err := state.AddID(ids.Empty, id); err != nil { + t.Fatal(err) + } } result, err = state.IDs(ids.Empty) @@ -115,14 +119,10 @@ func TestStateIDs(t *testing.T) { t.Fatalf("Should have returned the %s status", choices.Accepted) } - if err := state.SetIDs(ids.Empty, []ids.ID{ids.ID{}}); err == nil { + if err := state.AddID(ids.Empty, ids.ID{}); err == nil { t.Fatalf("Should have errored during serialization") } - if err := state.SetIDs(ids.Empty, []ids.ID{}); err != nil { - t.Fatal(err) - } - if _, err := state.IDs(ids.Empty); err == nil { t.Fatalf("Should have errored when reading ids") } @@ -153,7 +153,7 @@ func TestStateStatuses(t *testing.T) { t.Fatalf("Should have returned the %s status", choices.Accepted) } - if err := state.SetIDs(ids.Empty, []ids.ID{ids.Empty}); err != nil { + if err := state.AddID(ids.Empty, ids.Empty); err != nil { t.Fatal(err) } if _, err := state.Status(ids.Empty); err == nil { diff --git a/vms/components/ava/prefixed_state.go b/vms/components/ava/prefixed_state.go index dd7f3e85c39..2fcd9aae17b 100644 --- a/vms/components/ava/prefixed_state.go +++ b/vms/components/ava/prefixed_state.go @@ -97,11 +97,7 @@ func (s *chainState) setStatus(id ids.ID, status choices.Status) error { func (s *chainState) removeUTXO(addrs [][]byte, utxoID ids.ID) error { for _, addr := range addrs { addrID := ids.NewID(hashing.ComputeHash256Array(addr)) - utxos := ids.Set{} - funds, _ := s.Funds(addrID) - utxos.Add(funds...) - utxos.Remove(utxoID) - if err := s.setFunds(addrID, utxos.List()); err != nil { + if err := s.RemoveID(addrID, utxoID); err != nil { return err } } @@ -111,21 +107,13 @@ func (s *chainState) removeUTXO(addrs [][]byte, utxoID ids.ID) error { func (s *chainState) addUTXO(addrs [][]byte, utxoID ids.ID) error { for _, addr := range addrs { addrID := ids.NewID(hashing.ComputeHash256Array(addr)) - utxos := ids.Set{} - funds, _ := s.Funds(addrID) - utxos.Add(funds...) - utxos.Add(utxoID) - if err := s.setFunds(addrID, utxos.List()); err != nil { + if err := s.AddID(addrID, utxoID); err != nil { return err } } return nil } -func (s *chainState) setFunds(id ids.ID, idSlice []ids.ID) error { - return s.SetIDs(UniqueID(id, s.fundsIDPrefix, s.fundsID), idSlice) -} - // PrefixedState wraps a state object. By prefixing the state, there will // be no collisions between different types of objects that have the same hash. type PrefixedState struct { diff --git a/vms/components/ava/state.go b/vms/components/ava/state.go index 627cc5650af..8c8129fdd34 100644 --- a/vms/components/ava/state.go +++ b/vms/components/ava/state.go @@ -8,6 +8,7 @@ import ( "github.com/ava-labs/gecko/cache" "github.com/ava-labs/gecko/database" + "github.com/ava-labs/gecko/database/prefixdb" "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/choices" "github.com/ava-labs/gecko/vms/components/codec" @@ -116,39 +117,29 @@ func (s *State) SetStatus(id ids.ID, status choices.Status) error { // IDs returns a slice of IDs from storage func (s *State) IDs(id ids.ID) ([]ids.ID, error) { - if idsIntf, found := s.Cache.Get(id); found { - if idSlice, ok := idsIntf.([]ids.ID); ok { - return idSlice, nil - } - return nil, errCacheTypeMismatch - } + idSlice := []ids.ID(nil) + iter := prefixdb.New(id.Bytes(), s.DB).NewIterator() + defer iter.Release() - bytes, err := s.DB.Get(id.Bytes()) - if err != nil { - return nil, err - } + for iter.Next() { + keyID, err := ids.ToID(iter.Key()) + if err != nil { + return nil, err + } - idSlice := []ids.ID(nil) - if err := s.Codec.Unmarshal(bytes, &idSlice); err != nil { - return nil, err + idSlice = append(idSlice, keyID) } - - s.Cache.Put(id, idSlice) return idSlice, nil } -// SetIDs saves a slice of IDs to the database. -func (s *State) SetIDs(id ids.ID, idSlice []ids.ID) error { - if len(idSlice) == 0 { - s.Cache.Evict(id) - return s.DB.Delete(id.Bytes()) - } - - bytes, err := s.Codec.Marshal(idSlice) - if err != nil { - return err - } +// AddID saves an ID to the prefixed database +func (s *State) AddID(id ids.ID, key ids.ID) error { + db := prefixdb.New(id.Bytes(), s.DB) + return db.Put(key.Bytes(), nil) +} - s.Cache.Put(id, idSlice) - return s.DB.Put(id.Bytes(), bytes) +// RemoveID removes an ID from the prefixed database +func (s *State) RemoveID(id ids.ID, key ids.ID) error { + db := prefixdb.New(id.Bytes(), s.DB) + return db.Delete(key.Bytes()) } From 63181868488654378b5799bbd0938507127beba2 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Thu, 28 May 2020 23:48:08 -0400 Subject: [PATCH 06/29] wip currently has a deadlock error --- chains/manager.go | 5 +- snow/engine/avalanche/bootstrapper.go | 53 ++-- snow/engine/avalanche/bootstrapper_test.go | 15 +- snow/engine/avalanche/convincer.go | 4 +- snow/engine/avalanche/issuer.go | 9 +- snow/engine/avalanche/transitive.go | 134 ++++++---- snow/engine/avalanche/transitive_test.go | 2 +- snow/engine/avalanche/vertex_job.go | 3 +- snow/engine/avalanche/voter.go | 14 +- snow/engine/common/bootstrapable.go | 5 +- snow/engine/common/bootstrapper.go | 54 ++-- snow/engine/common/engine.go | 64 +++-- snow/engine/common/test_bootstrapable.go | 15 +- snow/engine/common/test_engine.go | 242 +++++++++++------- snow/engine/common/test_vm.go | 14 +- snow/engine/common/vm.go | 2 +- snow/engine/snowman/bootstrapper.go | 46 ++-- snow/engine/snowman/bootstrapper_test.go | 13 +- snow/engine/snowman/convincer.go | 4 +- snow/engine/snowman/issuer.go | 4 +- snow/engine/snowman/transitive.go | 146 +++++++---- snow/engine/snowman/transitive_test.go | 2 +- snow/engine/snowman/voter.go | 7 +- .../networking/{handler => router}/handler.go | 75 ++++-- .../networking/{handler => router}/message.go | 2 +- snow/networking/router/router.go | 3 +- snow/networking/router/subnet_router.go | 28 +- snow/networking/sender/sender_test.go | 14 +- vms/avm/vm.go | 8 +- vms/components/core/snowman_vm.go | 17 +- vms/platformvm/vm.go | 8 +- vms/platformvm/vm_test.go | 13 +- vms/rpcchainvm/vm_client.go | 13 +- vms/rpcchainvm/vm_server.go | 4 +- vms/spchainvm/consensus_benchmark_test.go | 23 +- vms/spchainvm/vm.go | 9 +- vms/spdagvm/vm.go | 9 +- 37 files changed, 655 insertions(+), 428 deletions(-) rename snow/networking/{handler => router}/handler.go (78%) rename snow/networking/{handler => router}/message.go (99%) diff --git a/chains/manager.go b/chains/manager.go index 49bdef54964..552bdea9891 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -20,7 +20,6 @@ import ( "github.com/ava-labs/gecko/snow/engine/avalanche/state" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/snow/engine/common/queue" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/router" "github.com/ava-labs/gecko/snow/networking/sender" "github.com/ava-labs/gecko/snow/networking/timeout" @@ -428,7 +427,7 @@ func (m *manager) createAvalancheChain( }) // Asynchronously passes messages from the network to the consensus engine - handler := &handler.Handler{} + handler := &router.Handler{} handler.Initialize(&engine, msgChan, defaultChannelSize) // Allows messages to be routed to the new chain @@ -514,7 +513,7 @@ func (m *manager) createSnowmanChain( }) // Asynchronously passes messages from the network to the consensus engine - handler := &handler.Handler{} + handler := &router.Handler{} handler.Initialize(&engine, msgChan, defaultChannelSize) // Allow incoming messages to be routed to the new chain diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index 78d7a330780..9a8ae9e2f50 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -42,11 +42,11 @@ type bootstrapper struct { // IDs of vertices that we have requested from other validators but haven't received pending ids.Set finished bool - onFinished func() + onFinished func() error } // Initialize this engine. -func (b *bootstrapper) Initialize(config BootstrapConfig) { +func (b *bootstrapper) Initialize(config BootstrapConfig) error { b.BootstrapConfig = config b.VtxBlocked.SetParser(&vtxParser{ @@ -65,6 +65,7 @@ func (b *bootstrapper) Initialize(config BootstrapConfig) { config.Bootstrapable = b b.Bootstrapper.Initialize(config.Config) + return nil } // CurrentAcceptedFrontier ... @@ -86,7 +87,7 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { } // ForceAccepted ... -func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) { +func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) error { for _, vtxID := range acceptedContainerIDs.List() { b.fetch(vtxID) } @@ -94,20 +95,20 @@ func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) { if numPending := b.pending.Len(); numPending == 0 { // TODO: This typically indicates bootstrapping has failed, so this // should be handled appropriately - b.finish() + return b.finish() } + return nil } // Put ... -func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxBytes []byte) { +func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxBytes []byte) error { vtx, err := b.State.ParseVertex(vtxBytes) if err != nil { b.BootstrapConfig.Context.Log.Debug("ParseVertex failed due to %s for block:\n%s", err, formatting.DumpBytes{Bytes: vtxBytes}) - b.GetFailed(vdr, requestID) - return + return b.GetFailed(vdr, requestID) } if !b.pending.Contains(vtx.ID()) { @@ -115,23 +116,23 @@ func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxB vdr, formatting.DumpBytes{Bytes: vtxBytes}) - b.GetFailed(vdr, requestID) - return + return b.GetFailed(vdr, requestID) } - b.addVertex(vtx) + return b.addVertex(vtx) } // GetFailed ... -func (b *bootstrapper) GetFailed(vdr ids.ShortID, requestID uint32) { +func (b *bootstrapper) GetFailed(vdr ids.ShortID, requestID uint32) error { vtxID, ok := b.vtxReqs.Remove(vdr, requestID) if !ok { b.BootstrapConfig.Context.Log.Debug("GetFailed called without sending the corresponding Get message from %s", vdr) - return + return nil } b.sendRequest(vtxID) + return nil } func (b *bootstrapper) fetch(vtxID ids.ID) { @@ -165,12 +166,13 @@ func (b *bootstrapper) sendRequest(vtxID ids.ID) { b.numBSPendingRequests.Set(float64(b.pending.Len())) } -func (b *bootstrapper) addVertex(vtx avalanche.Vertex) { +func (b *bootstrapper) addVertex(vtx avalanche.Vertex) error { b.storeVertex(vtx) if numPending := b.pending.Len(); numPending == 0 { - b.finish() + return b.finish() } + return nil } func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { @@ -231,27 +233,36 @@ func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { b.numBSPendingRequests.Set(float64(numPending)) } -func (b *bootstrapper) finish() { +func (b *bootstrapper) finish() error { if b.finished { - return + return nil } b.BootstrapConfig.Context.Log.Info("bootstrapping finished fetching vertices. executing state transitions...") - b.executeAll(b.TxBlocked, b.numBSBlockedTx) - b.executeAll(b.VtxBlocked, b.numBSBlockedVtx) + if err := b.executeAll(b.TxBlocked, b.numBSBlockedTx); err != nil { + return err + } + if err := b.executeAll(b.VtxBlocked, b.numBSBlockedVtx); err != nil { + return err + } // Start consensus - b.onFinished() + if err := b.onFinished(); err != nil { + return err + } b.seen = ids.Set{} b.finished = true + return nil } -func (b *bootstrapper) executeAll(jobs *queue.Jobs, numBlocked prometheus.Gauge) { +func (b *bootstrapper) executeAll(jobs *queue.Jobs, numBlocked prometheus.Gauge) error { for job, err := jobs.Pop(); err == nil; job, err = jobs.Pop() { numBlocked.Dec() b.BootstrapConfig.Context.Log.Debug("Executing: %s", job.ID()) if err := jobs.Execute(job); err != nil { - b.BootstrapConfig.Context.Log.Warn("Error executing: %s", err) + b.BootstrapConfig.Context.Log.Error("Error executing: %s", err) + return err } } + return nil } diff --git a/snow/engine/avalanche/bootstrapper_test.go b/snow/engine/avalanche/bootstrapper_test.go index c477e050b6e..f5cc3b50bf5 100644 --- a/snow/engine/avalanche/bootstrapper_test.go +++ b/snow/engine/avalanche/bootstrapper_test.go @@ -21,7 +21,6 @@ import ( "github.com/ava-labs/gecko/snow/consensus/snowstorm" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/snow/engine/common/queue" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/router" "github.com/ava-labs/gecko/snow/networking/timeout" "github.com/ava-labs/gecko/snow/validators" @@ -41,7 +40,7 @@ func newConfig(t *testing.T) (BootstrapConfig, ids.ShortID, *common.SenderTest, state := &stateTest{} vm := &VMTest{} engine := &Transitive{} - handler := &handler.Handler{} + handler := &router.Handler{} router := &router.ChainRouter{} timeouts := &timeout.Manager{} @@ -196,7 +195,7 @@ func TestBootstrapperSingleFrontier(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } for vtxKey, reqID := range vtxIDToReqID { vtxID := ids.NewID(vtxKey) @@ -314,7 +313,7 @@ func TestBootstrapperUnknownByzantineResponse(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *requestID, vtxID1, vtxBytes1) bs.Put(peerID, *requestID, vtxID0, vtxBytes0) @@ -461,7 +460,7 @@ func TestBootstrapperVertexDependencies(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *reqIDPtr, vtxID0, vtxBytes0) @@ -653,7 +652,7 @@ func TestBootstrapperTxDependencies(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *reqIDPtr, vtxID0, vtxBytes0) @@ -845,7 +844,7 @@ func TestBootstrapperMissingTxDependency(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *reqIDPtr, vtxID0, vtxBytes0) @@ -1092,7 +1091,7 @@ func TestBootstrapperWrongIDByzantineResponse(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } sender.CantGet = false bs.Put(peerID, *requestID, vtxID0, vtxBytes1) diff --git a/snow/engine/avalanche/convincer.go b/snow/engine/avalanche/convincer.go index 98736613bf1..1635ae7346d 100644 --- a/snow/engine/avalanche/convincer.go +++ b/snow/engine/avalanche/convincer.go @@ -7,6 +7,7 @@ import ( "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/consensus/avalanche" "github.com/ava-labs/gecko/snow/engine/common" + "github.com/ava-labs/gecko/utils/wrappers" ) type convincer struct { @@ -16,6 +17,7 @@ type convincer struct { requestID uint32 abandoned bool deps ids.Set + errs *wrappers.Errs } func (c *convincer) Dependencies() ids.Set { return c.deps } @@ -28,7 +30,7 @@ func (c *convincer) Fulfill(id ids.ID) { func (c *convincer) Abandon(ids.ID) { c.abandoned = true } func (c *convincer) Update() { - if c.abandoned || c.deps.Len() != 0 { + if c.abandoned || c.deps.Len() != 0 || c.errs.Errored() { return } diff --git a/snow/engine/avalanche/issuer.go b/snow/engine/avalanche/issuer.go index 57e2179ef51..4cbc408ffe1 100644 --- a/snow/engine/avalanche/issuer.go +++ b/snow/engine/avalanche/issuer.go @@ -37,7 +37,7 @@ func (i *issuer) Abandon() { } func (i *issuer) Update() { - if i.abandoned || i.issued || i.vtxDeps.Len() != 0 || i.txDeps.Len() != 0 || i.t.Consensus.VertexIssued(i.vtx) { + if i.abandoned || i.issued || i.vtxDeps.Len() != 0 || i.txDeps.Len() != 0 || i.t.Consensus.VertexIssued(i.vtx) || i.t.errs.Errored() { return } i.issued = true @@ -65,7 +65,10 @@ func (i *issuer) Update() { i.t.Config.Context.Log.Verbo("Adding vertex to consensus:\n%s", i.vtx) - i.t.Consensus.Add(i.vtx) + if err := i.t.Consensus.Add(i.vtx); err != nil { + i.t.errs.Add(err) + return + } p := i.t.Consensus.Parameters() vdrs := i.t.Config.Validators.Sample(p.K) // Validators to sample @@ -87,7 +90,7 @@ func (i *issuer) Update() { i.t.txBlocked.Fulfill(tx.ID()) } - i.t.repoll() + i.t.errs.Add(i.t.repoll()) } type vtxIssuer struct{ i *issuer } diff --git a/snow/engine/avalanche/transitive.go b/snow/engine/avalanche/transitive.go index c8a5d33cf38..15f659e2cd0 100644 --- a/snow/engine/avalanche/transitive.go +++ b/snow/engine/avalanche/transitive.go @@ -12,6 +12,7 @@ import ( "github.com/ava-labs/gecko/snow/events" "github.com/ava-labs/gecko/utils/formatting" "github.com/ava-labs/gecko/utils/random" + "github.com/ava-labs/gecko/utils/wrappers" ) // Transitive implements the Engine interface by attempting to fetch all @@ -33,24 +34,27 @@ type Transitive struct { vtxBlocked, txBlocked events.Blocker bootstrapped bool + + errs wrappers.Errs } // Initialize implements the Engine interface -func (t *Transitive) Initialize(config Config) { +func (t *Transitive) Initialize(config Config) error { config.Context.Log.Info("Initializing Avalanche consensus") t.Config = config t.metrics.Initialize(config.Context.Log, config.Params.Namespace, config.Params.Metrics) t.onFinished = t.finishBootstrapping - t.bootstrapper.Initialize(config.BootstrapConfig) t.polls.log = config.Context.Log t.polls.numPolls = t.numPolls t.polls.m = make(map[uint32]poll) + + return t.bootstrapper.Initialize(config.BootstrapConfig) } -func (t *Transitive) finishBootstrapping() { +func (t *Transitive) finishBootstrapping() error { // Load the vertices that were last saved as the accepted frontier frontier := []avalanche.Vertex(nil) for _, vtxID := range t.Config.State.Edge() { @@ -64,14 +68,15 @@ func (t *Transitive) finishBootstrapping() { t.bootstrapped = true t.Config.Context.Log.Info("Bootstrapping finished with %d vertices in the accepted frontier", len(frontier)) + return nil } // Gossip implements the Engine interface -func (t *Transitive) Gossip() { +func (t *Transitive) Gossip() error { edge := t.Config.State.Edge() if len(edge) == 0 { t.Config.Context.Log.Debug("Dropping gossip request as no vertices have been accepted") - return + return nil } sampler := random.Uniform{N: len(edge)} @@ -79,37 +84,38 @@ func (t *Transitive) Gossip() { vtx, err := t.Config.State.GetVertex(vtxID) if err != nil { t.Config.Context.Log.Warn("Dropping gossip request as %s couldn't be loaded due to %s", vtxID, err) - return + return nil } t.Config.Context.Log.Debug("Gossiping %s as accepted to the network", vtxID) t.Config.Sender.Gossip(vtxID, vtx.Bytes()) + return nil } // Shutdown implements the Engine interface -func (t *Transitive) Shutdown() { +func (t *Transitive) Shutdown() error { t.Config.Context.Log.Info("Shutting down Avalanche consensus") - t.Config.VM.Shutdown() + return t.Config.VM.Shutdown() } // Context implements the Engine interface func (t *Transitive) Context() *snow.Context { return t.Config.Context } // Get implements the Engine interface -func (t *Transitive) Get(vdr ids.ShortID, requestID uint32, vtxID ids.ID) { +func (t *Transitive) Get(vdr ids.ShortID, requestID uint32, vtxID ids.ID) error { // If this engine has access to the requested vertex, provide it if vtx, err := t.Config.State.GetVertex(vtxID); err == nil { t.Config.Sender.Put(vdr, requestID, vtxID, vtx.Bytes()) } + return nil } // Put implements the Engine interface -func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxBytes []byte) { +func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxBytes []byte) error { t.Config.Context.Log.Verbo("Put called for vertexID %s", vtxID) if !t.bootstrapped { - t.bootstrapper.Put(vdr, requestID, vtxID, vtxBytes) - return + return t.bootstrapper.Put(vdr, requestID, vtxID, vtxBytes) } vtx, err := t.Config.State.ParseVertex(vtxBytes) @@ -117,24 +123,23 @@ func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxByt t.Config.Context.Log.Debug("ParseVertex failed due to %s for block:\n%s", err, formatting.DumpBytes{Bytes: vtxBytes}) - t.GetFailed(vdr, requestID) - return + return t.GetFailed(vdr, requestID) } - t.insertFrom(vdr, vtx) + _, err = t.insertFrom(vdr, vtx) + return err } // GetFailed implements the Engine interface -func (t *Transitive) GetFailed(vdr ids.ShortID, requestID uint32) { +func (t *Transitive) GetFailed(vdr ids.ShortID, requestID uint32) error { if !t.bootstrapped { - t.bootstrapper.GetFailed(vdr, requestID) - return + return t.bootstrapper.GetFailed(vdr, requestID) } vtxID, ok := t.vtxReqs.Remove(vdr, requestID) if !ok { t.Config.Context.Log.Warn("GetFailed called without sending the corresponding Get message from %s", vdr) - return + return nil } t.vtxBlocked.Abandon(vtxID) @@ -149,13 +154,14 @@ func (t *Transitive) GetFailed(vdr ids.ShortID, requestID uint32) { // Track performance statistics t.numVtxRequests.Set(float64(t.vtxReqs.Len())) t.numTxRequests.Set(float64(t.missingTxs.Len())) + return t.errs.Err } // PullQuery implements the Engine interface -func (t *Transitive) PullQuery(vdr ids.ShortID, requestID uint32, vtxID ids.ID) { +func (t *Transitive) PullQuery(vdr ids.ShortID, requestID uint32, vtxID ids.ID) error { if !t.bootstrapped { t.Config.Context.Log.Debug("Dropping PullQuery for %s due to bootstrapping", vtxID) - return + return nil } c := &convincer{ @@ -163,20 +169,27 @@ func (t *Transitive) PullQuery(vdr ids.ShortID, requestID uint32, vtxID ids.ID) sender: t.Config.Sender, vdr: vdr, requestID: requestID, + errs: &t.errs, + } + + added, err := t.reinsertFrom(vdr, vtxID) + if err != nil { + return err } - if !t.reinsertFrom(vdr, vtxID) { + if !added { c.deps.Add(vtxID) } t.vtxBlocked.Register(c) + return t.errs.Err } // PushQuery implements the Engine interface -func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxBytes []byte) { +func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, vtxID ids.ID, vtxBytes []byte) error { if !t.bootstrapped { t.Config.Context.Log.Debug("Dropping PushQuery for %s due to bootstrapping", vtxID) - return + return nil } vtx, err := t.Config.State.ParseVertex(vtxBytes) @@ -184,18 +197,21 @@ func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, vtxID ids.ID, t.Config.Context.Log.Warn("ParseVertex failed due to %s for block:\n%s", err, formatting.DumpBytes{Bytes: vtxBytes}) - return + return nil + } + + if _, err := t.insertFrom(vdr, vtx); err != nil { + return err } - t.insertFrom(vdr, vtx) - t.PullQuery(vdr, requestID, vtx.ID()) + return t.PullQuery(vdr, requestID, vtx.ID()) } // Chits implements the Engine interface -func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) { +func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) error { if !t.bootstrapped { t.Config.Context.Log.Debug("Dropping Chits due to bootstrapping") - return + return nil } v := &voter{ @@ -206,56 +222,65 @@ func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) { } voteList := votes.List() for _, vote := range voteList { - if !t.reinsertFrom(vdr, vote) { + if added, err := t.reinsertFrom(vdr, vote); err != nil { + return err + } else if !added { v.deps.Add(vote) } } t.vtxBlocked.Register(v) + return t.errs.Err } // QueryFailed implements the Engine interface -func (t *Transitive) QueryFailed(vdr ids.ShortID, requestID uint32) { - t.Chits(vdr, requestID, ids.Set{}) +func (t *Transitive) QueryFailed(vdr ids.ShortID, requestID uint32) error { + return t.Chits(vdr, requestID, ids.Set{}) } // Notify implements the Engine interface -func (t *Transitive) Notify(msg common.Message) { +func (t *Transitive) Notify(msg common.Message) error { if !t.bootstrapped { t.Config.Context.Log.Warn("Dropping Notify due to bootstrapping") - return + return nil } switch msg { case common.PendingTxs: txs := t.Config.VM.PendingTxs() - t.batch(txs, false /*=force*/, false /*=empty*/) + return t.batch(txs, false /*=force*/, false /*=empty*/) } + return nil } -func (t *Transitive) repoll() { - if len(t.polls.m) >= t.Params.ConcurrentRepolls { - return +func (t *Transitive) repoll() error { + if len(t.polls.m) >= t.Params.ConcurrentRepolls || t.errs.Errored() { + return nil } txs := t.Config.VM.PendingTxs() - t.batch(txs, false /*=force*/, true /*=empty*/) + if err := t.batch(txs, false /*=force*/, true /*=empty*/); err != nil { + return err + } for i := len(t.polls.m); i < t.Params.ConcurrentRepolls; i++ { - t.batch(nil, false /*=force*/, true /*=empty*/) + if err := t.batch(nil, false /*=force*/, true /*=empty*/); err != nil { + return err + } } + return nil } -func (t *Transitive) reinsertFrom(vdr ids.ShortID, vtxID ids.ID) bool { +func (t *Transitive) reinsertFrom(vdr ids.ShortID, vtxID ids.ID) (bool, error) { vtx, err := t.Config.State.GetVertex(vtxID) if err != nil { t.sendRequest(vdr, vtxID) - return false + return false, nil } return t.insertFrom(vdr, vtx) } -func (t *Transitive) insertFrom(vdr ids.ShortID, vtx avalanche.Vertex) bool { +func (t *Transitive) insertFrom(vdr ids.ShortID, vtx avalanche.Vertex) (bool, error) { issued := true vts := []avalanche.Vertex{vtx} for len(vts) > 0 { @@ -279,12 +304,14 @@ func (t *Transitive) insertFrom(vdr ids.ShortID, vtx avalanche.Vertex) bool { } } - t.insert(vtx) + if err := t.insert(vtx); err != nil { + return false, err + } } - return issued + return issued, nil } -func (t *Transitive) insert(vtx avalanche.Vertex) { +func (t *Transitive) insert(vtx avalanche.Vertex) error { vtxID := vtx.ID() t.pending.Add(vtxID) @@ -334,9 +361,10 @@ func (t *Transitive) insert(vtx avalanche.Vertex) { t.numVtxRequests.Set(float64(t.vtxReqs.Len())) t.numTxRequests.Set(float64(t.missingTxs.Len())) t.numPendingVtx.Set(float64(t.pending.Len())) + return t.errs.Err } -func (t *Transitive) batch(txs []snowstorm.Tx, force, empty bool) { +func (t *Transitive) batch(txs []snowstorm.Tx, force, empty bool) error { batch := []snowstorm.Tx(nil) issuedTxs := ids.Set{} consumed := ids.Set{} @@ -361,10 +389,11 @@ func (t *Transitive) batch(txs []snowstorm.Tx, force, empty bool) { } if len(batch) > 0 { - t.issueBatch(batch) + return t.issueBatch(batch) } else if empty && !issued { t.issueRepoll() } + return nil } func (t *Transitive) issueRepoll() { @@ -394,7 +423,7 @@ func (t *Transitive) issueRepoll() { } } -func (t *Transitive) issueBatch(txs []snowstorm.Tx) { +func (t *Transitive) issueBatch(txs []snowstorm.Tx) error { t.Config.Context.Log.Verbo("Batching %d transactions into a new vertex", len(txs)) virtuousIDs := t.Consensus.Virtuous().List() @@ -404,11 +433,12 @@ func (t *Transitive) issueBatch(txs []snowstorm.Tx) { parentIDs.Add(virtuousIDs[sampler.Sample()]) } - if vtx, err := t.Config.State.BuildVertex(parentIDs, txs); err == nil { - t.insert(vtx) - } else { + vtx, err := t.Config.State.BuildVertex(parentIDs, txs) + if err != nil { t.Config.Context.Log.Warn("Error building new vertex with %d parents and %d transactions", len(parentIDs), len(txs)) + return nil } + return t.insert(vtx) } func (t *Transitive) sendRequest(vdr ids.ShortID, vtxID ids.ID) { diff --git a/snow/engine/avalanche/transitive_test.go b/snow/engine/avalanche/transitive_test.go index 7f27fe1da8f..31b8af8a1b6 100644 --- a/snow/engine/avalanche/transitive_test.go +++ b/snow/engine/avalanche/transitive_test.go @@ -28,7 +28,7 @@ func TestEngineShutdown(t *testing.T) { config := DefaultConfig() vmShutdownCalled := false vm := &VMTest{} - vm.ShutdownF = func() { vmShutdownCalled = true } + vm.ShutdownF = func() error { vmShutdownCalled = true; return nil } config.VM = vm transitive := &Transitive{} diff --git a/snow/engine/avalanche/vertex_job.go b/snow/engine/avalanche/vertex_job.go index bc023c13e46..956e24c047e 100644 --- a/snow/engine/avalanche/vertex_job.go +++ b/snow/engine/avalanche/vertex_job.go @@ -59,7 +59,8 @@ func (v *vertexJob) Execute() error { for _, tx := range v.vtx.Txs() { if tx.Status() != choices.Accepted { v.numDropped.Inc() - return errors.New("attempting to execute vertex with non-accepted transactions") + v.log.Warn("attempting to execute vertex with non-accepted transactions") + return nil } } status := v.vtx.Status() diff --git a/snow/engine/avalanche/voter.go b/snow/engine/avalanche/voter.go index b14ef404441..581fe27092d 100644 --- a/snow/engine/avalanche/voter.go +++ b/snow/engine/avalanche/voter.go @@ -27,7 +27,7 @@ func (v *voter) Fulfill(id ids.ID) { func (v *voter) Abandon(id ids.ID) { v.Fulfill(id) } func (v *voter) Update() { - if v.deps.Len() != 0 { + if v.deps.Len() != 0 || v.t.errs.Errored() { return } @@ -38,7 +38,10 @@ func (v *voter) Update() { results = v.bubbleVotes(results) v.t.Config.Context.Log.Debug("Finishing poll with:\n%s", &results) - v.t.Consensus.RecordPoll(results) + if err := v.t.Consensus.RecordPoll(results); err != nil { + v.t.errs.Add(err) + return + } txs := []snowstorm.Tx(nil) for _, orphanID := range v.t.Consensus.Orphans().List() { @@ -51,7 +54,10 @@ func (v *voter) Update() { if len(txs) > 0 { v.t.Config.Context.Log.Debug("Re-issuing %d transactions", len(txs)) } - v.t.batch(txs, true /*=force*/, false /*empty*/) + if err := v.t.batch(txs, true /*=force*/, false /*empty*/); err != nil { + v.t.errs.Add(err) + return + } if v.t.Consensus.Quiesce() { v.t.Config.Context.Log.Verbo("Avalanche engine can quiesce") @@ -59,7 +65,7 @@ func (v *voter) Update() { } v.t.Config.Context.Log.Verbo("Avalanche engine can't quiesce") - v.t.repoll() + v.t.errs.Add(v.t.repoll()) } func (v *voter) bubbleVotes(votes ids.UniqueBag) ids.UniqueBag { diff --git a/snow/engine/common/bootstrapable.go b/snow/engine/common/bootstrapable.go index d0ddc37c6c1..f59d55f06ef 100644 --- a/snow/engine/common/bootstrapable.go +++ b/snow/engine/common/bootstrapable.go @@ -16,6 +16,7 @@ type Bootstrapable interface { // Returns the subset of containerIDs that are accepted by this chain. FilterAccepted(containerIDs ids.Set) (acceptedContainerIDs ids.Set) - // Force the provided containers to be accepted. - ForceAccepted(acceptedContainerIDs ids.Set) + // Force the provided containers to be accepted. Only returns fatal errors + // if they occur. + ForceAccepted(acceptedContainerIDs ids.Set) error } diff --git a/snow/engine/common/bootstrapper.go b/snow/engine/common/bootstrapper.go index 61c191a88f5..82064f7d439 100644 --- a/snow/engine/common/bootstrapper.go +++ b/snow/engine/common/bootstrapper.go @@ -37,11 +37,10 @@ func (b *Bootstrapper) Initialize(config Config) { } // Startup implements the Engine interface. -func (b *Bootstrapper) Startup() { +func (b *Bootstrapper) Startup() error { if b.pendingAcceptedFrontier.Len() == 0 { b.Context.Log.Info("Bootstrapping skipped due to no provided bootstraps") - b.Bootstrapable.ForceAccepted(ids.Set{}) - return + return b.Bootstrapable.ForceAccepted(ids.Set{}) } vdrs := ids.ShortSet{} @@ -49,23 +48,26 @@ func (b *Bootstrapper) Startup() { b.RequestID++ b.Sender.GetAcceptedFrontier(vdrs, b.RequestID) + return nil } // GetAcceptedFrontier implements the Engine interface. -func (b *Bootstrapper) GetAcceptedFrontier(validatorID ids.ShortID, requestID uint32) { +func (b *Bootstrapper) GetAcceptedFrontier(validatorID ids.ShortID, requestID uint32) error { b.Sender.AcceptedFrontier(validatorID, requestID, b.Bootstrapable.CurrentAcceptedFrontier()) + return nil } // GetAcceptedFrontierFailed implements the Engine interface. -func (b *Bootstrapper) GetAcceptedFrontierFailed(validatorID ids.ShortID, requestID uint32) { +func (b *Bootstrapper) GetAcceptedFrontierFailed(validatorID ids.ShortID, requestID uint32) error { b.AcceptedFrontier(validatorID, requestID, ids.Set{}) + return nil } // AcceptedFrontier implements the Engine interface. -func (b *Bootstrapper) AcceptedFrontier(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (b *Bootstrapper) AcceptedFrontier(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { if !b.pendingAcceptedFrontier.Contains(validatorID) { b.Context.Log.Debug("Received an AcceptedFrontier message from %s unexpectedly", validatorID) - return + return nil } b.pendingAcceptedFrontier.Remove(validatorID) @@ -78,23 +80,25 @@ func (b *Bootstrapper) AcceptedFrontier(validatorID ids.ShortID, requestID uint3 b.RequestID++ b.Sender.GetAccepted(vdrs, b.RequestID, b.acceptedFrontier) } + return nil } // GetAccepted implements the Engine interface. -func (b *Bootstrapper) GetAccepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (b *Bootstrapper) GetAccepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { b.Sender.Accepted(validatorID, requestID, b.Bootstrapable.FilterAccepted(containerIDs)) + return nil } // GetAcceptedFailed implements the Engine interface. -func (b *Bootstrapper) GetAcceptedFailed(validatorID ids.ShortID, requestID uint32) { - b.Accepted(validatorID, requestID, ids.Set{}) +func (b *Bootstrapper) GetAcceptedFailed(validatorID ids.ShortID, requestID uint32) error { + return b.Accepted(validatorID, requestID, ids.Set{}) } // Accepted implements the Engine interface. -func (b *Bootstrapper) Accepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (b *Bootstrapper) Accepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { if !b.pendingAccepted.Contains(validatorID) { b.Context.Log.Debug("Received an Accepted message from %s unexpectedly", validatorID) - return + return nil } b.pendingAccepted.Remove(validatorID) @@ -113,20 +117,22 @@ func (b *Bootstrapper) Accepted(validatorID ids.ShortID, requestID uint32, conta b.acceptedVotes[key] = newWeight } - if b.pendingAccepted.Len() == 0 { - accepted := ids.Set{} - for key, weight := range b.acceptedVotes { - if weight >= b.Config.Alpha { - accepted.Add(ids.NewID(key)) - } - } + if b.pendingAccepted.Len() != 0 { + return nil + } - if size := accepted.Len(); size == 0 && b.Config.Beacons.Len() > 0 { - b.Context.Log.Warn("Bootstrapping finished with no accepted frontier. This is likely a result of failing to be able to connect to the specified bootstraps, or no transactions have been issued on this chain yet") - } else { - b.Context.Log.Info("Bootstrapping started syncing with %d vertices in the accepted frontier", size) + accepted := ids.Set{} + for key, weight := range b.acceptedVotes { + if weight >= b.Config.Alpha { + accepted.Add(ids.NewID(key)) } + } - b.Bootstrapable.ForceAccepted(accepted) + if size := accepted.Len(); size == 0 && b.Config.Beacons.Len() > 0 { + b.Context.Log.Info("Bootstrapping finished with no accepted frontier. This is likely a result of failing to be able to connect to the specified bootstraps, or no transactions have been issued on this chain yet") + } else { + b.Context.Log.Info("Bootstrapping started syncing with %d vertices in the accepted frontier", size) } + + return b.Bootstrapable.ForceAccepted(accepted) } diff --git a/snow/engine/common/engine.go b/snow/engine/common/engine.go index d0d41c0959d..6b1da6ca8da 100644 --- a/snow/engine/common/engine.go +++ b/snow/engine/common/engine.go @@ -32,7 +32,7 @@ type ExternalHandler interface { } // FrontierHandler defines how a consensus engine reacts to frontier messages -// from other validators +// from other validators. Functions only return fatal errors if they occur. type FrontierHandler interface { // Notify this engine of a request for the accepted frontier of vertices. // @@ -45,19 +45,19 @@ type FrontierHandler interface { // // This engine should respond with an AcceptedFrontier message with the same // requestID, and the engine's current accepted frontier. - GetAcceptedFrontier(validatorID ids.ShortID, requestID uint32) + GetAcceptedFrontier(validatorID ids.ShortID, requestID uint32) error // Notify this engine of an accepted frontier. // // This function can be called by any validator. It is not safe to assume - // this message is in response to a GetAcceptedFrontier message, is utilizing a - // unique requestID, or that the containerIDs from a valid frontier. - // However, the validatorID is assumed to be authenticated. + // this message is in response to a GetAcceptedFrontier message, is + // utilizing a unique requestID, or that the containerIDs from a valid + // frontier. However, the validatorID is assumed to be authenticated. AcceptedFrontier( validatorID ids.ShortID, requestID uint32, containerIDs ids.Set, - ) + ) error // Notify this engine that a get accepted frontier request it issued has // failed. @@ -69,11 +69,12 @@ type FrontierHandler interface { // // The validatorID, and requestID, are assumed to be the same as those sent // in the GetAcceptedFrontier message. - GetAcceptedFrontierFailed(validatorID ids.ShortID, requestID uint32) + GetAcceptedFrontierFailed(validatorID ids.ShortID, requestID uint32) error } // AcceptedHandler defines how a consensus engine reacts to messages pertaining -// to accepted containers from other validators +// to accepted containers from other validators. Functions only return fatal +// errors if they occur. type AcceptedHandler interface { // Notify this engine of a request to filter non-accepted vertices. // @@ -84,7 +85,11 @@ type AcceptedHandler interface { // This engine should respond with an Accepted message with the same // requestID, and the subset of the containerIDs that this node has decided // are accepted. - GetAccepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) + GetAccepted( + validatorID ids.ShortID, + requestID uint32, + containerIDs ids.Set, + ) error // Notify this engine of a set of accepted vertices. // @@ -93,7 +98,11 @@ type AcceptedHandler interface { // unique requestID, or that the containerIDs are a subset of the // containerIDs from a GetAccepted message. However, the validatorID is // assumed to be authenticated. - Accepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) + Accepted( + validatorID ids.ShortID, + requestID uint32, + containerIDs ids.Set, + ) error // Notify this engine that a get accepted request it issued has failed. // @@ -104,11 +113,11 @@ type AcceptedHandler interface { // // The validatorID, and requestID, are assumed to be the same as those sent // in the GetAccepted message. - GetAcceptedFailed(validatorID ids.ShortID, requestID uint32) + GetAcceptedFailed(validatorID ids.ShortID, requestID uint32) error } // FetchHandler defines how a consensus engine reacts to retrieval messages from -// other validators +// other validators. Functions only return fatal errors if they occur. type FetchHandler interface { // Notify this engine of a request for a container. // @@ -124,7 +133,7 @@ type FetchHandler interface { // This engine should respond with a Put message with the same requestID if // the container was locally avaliable. Otherwise, the message can be safely // dropped. - Get(validatorID ids.ShortID, requestID uint32, containerID ids.ID) + Get(validatorID ids.ShortID, requestID uint32, containerID ids.ID) error // Notify this engine of a container. // @@ -141,7 +150,7 @@ type FetchHandler interface { requestID uint32, containerID ids.ID, container []byte, - ) + ) error // Notify this engine that a get request it issued has failed. // @@ -151,11 +160,11 @@ type FetchHandler interface { // // The validatorID and requestID are assumed to be the same as those sent in // the Get message. - GetFailed(validatorID ids.ShortID, requestID uint32) + GetFailed(validatorID ids.ShortID, requestID uint32) error } // QueryHandler defines how a consensus engine reacts to query messages from -// other validators +// other validators. Functions only return fatal errors if they occur. type QueryHandler interface { // Notify this engine of a request for our preferences. // @@ -168,7 +177,11 @@ type QueryHandler interface { // is complete, this engine should send this validator the current // preferences in a Chits message. The Chits message should have the same // requestID that was passed in here. - PullQuery(validatorID ids.ShortID, requestID uint32, containerID ids.ID) + PullQuery( + validatorID ids.ShortID, + requestID uint32, + containerID ids.ID, + ) error // Notify this engine of a request for our preferences. // @@ -191,14 +204,14 @@ type QueryHandler interface { requestID uint32, containerID ids.ID, container []byte, - ) + ) error // Notify this engine of the specified validators preferences. // // This function can be called by any validator. It is not safe to assume // this message is in response to a PullQuery or a PushQuery message. // However, the validatorID is assumed to be authenticated. - Chits(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) + Chits(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error // Notify this engine that a query it issued has failed. // @@ -209,26 +222,27 @@ type QueryHandler interface { // // The validatorID and the requestID are assumed to be the same as those // sent in the Query message. - QueryFailed(validatorID ids.ShortID, requestID uint32) + QueryFailed(validatorID ids.ShortID, requestID uint32) error } // InternalHandler defines how this consensus engine reacts to messages from -// other components of this validator +// other components of this validator. Functions only return fatal errors if +// they occur. type InternalHandler interface { // Startup this engine. // // This function will be called once the environment is configured to be // able to run the engine. - Startup() + Startup() error // Gossip to the network a container on the accepted frontier - Gossip() + Gossip() error // Shutdown this engine. // // This function will be called when the environment is exiting. - Shutdown() + Shutdown() error // Notify this engine of a message from the virtual machine. - Notify(Message) + Notify(Message) error } diff --git a/snow/engine/common/test_bootstrapable.go b/snow/engine/common/test_bootstrapable.go index 79698c6cf33..b9eac91c81d 100644 --- a/snow/engine/common/test_bootstrapable.go +++ b/snow/engine/common/test_bootstrapable.go @@ -4,6 +4,7 @@ package common import ( + "errors" "testing" "github.com/ava-labs/gecko/ids" @@ -19,7 +20,7 @@ type BootstrapableTest struct { CurrentAcceptedFrontierF func() (acceptedContainerIDs ids.Set) FilterAcceptedF func(containerIDs ids.Set) (acceptedContainerIDs ids.Set) - ForceAcceptedF func(acceptedContainerIDs ids.Set) + ForceAcceptedF func(acceptedContainerIDs ids.Set) error } // Default sets the default on call handling @@ -52,10 +53,14 @@ func (b *BootstrapableTest) FilterAccepted(containerIDs ids.Set) ids.Set { } // ForceAccepted implements the Bootstrapable interface -func (b *BootstrapableTest) ForceAccepted(containerIDs ids.Set) { +func (b *BootstrapableTest) ForceAccepted(containerIDs ids.Set) error { if b.ForceAcceptedF != nil { - b.ForceAcceptedF(containerIDs) - } else if b.CantForceAccepted && b.T != nil { - b.T.Fatalf("Unexpectedly called ForceAccepted") + return b.ForceAcceptedF(containerIDs) + } else if b.CantForceAccepted { + if b.T != nil { + b.T.Fatalf("Unexpectedly called ForceAccepted") + } + return errors.New("Unexpectedly called ForceAccepted") } + return nil } diff --git a/snow/engine/common/test_engine.go b/snow/engine/common/test_engine.go index 1bbd06090cb..5dafc241c5c 100644 --- a/snow/engine/common/test_engine.go +++ b/snow/engine/common/test_engine.go @@ -4,6 +4,7 @@ package common import ( + "errors" "testing" "github.com/ava-labs/gecko/ids" @@ -39,16 +40,17 @@ type EngineTest struct { CantQueryFailed, CantChits bool - StartupF, GossipF, ShutdownF func() - ContextF func() *snow.Context - NotifyF func(Message) - GetF, PullQueryF func(validatorID ids.ShortID, requestID uint32, containerID ids.ID) - GetFailedF func(validatorID ids.ShortID, requestID uint32) - PutF, PushQueryF func(validatorID ids.ShortID, requestID uint32, containerID ids.ID, container []byte) - GetAcceptedFrontierF, GetAcceptedFrontierFailedF, GetAcceptedFailedF, QueryFailedF func(validatorID ids.ShortID, requestID uint32) - AcceptedFrontierF, GetAcceptedF, AcceptedF, ChitsF func(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) + ContextF func() *snow.Context + StartupF, GossipF, ShutdownF func() error + NotifyF func(Message) error + GetF, PullQueryF func(validatorID ids.ShortID, requestID uint32, containerID ids.ID) error + PutF, PushQueryF func(validatorID ids.ShortID, requestID uint32, containerID ids.ID, container []byte) error + AcceptedFrontierF, GetAcceptedF, AcceptedF, ChitsF func(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error + GetAcceptedFrontierF, GetFailedF, QueryFailedF, GetAcceptedFrontierFailedF, GetAcceptedFailedF func(validatorID ids.ShortID, requestID uint32) error } +var _ Engine = &EngineTest{} + // Default ... func (e *EngineTest) Default(cant bool) { e.CantStartup = cant @@ -77,166 +79,234 @@ func (e *EngineTest) Default(cant bool) { e.CantChits = cant } +// Context ... +func (e *EngineTest) Context() *snow.Context { + if e.ContextF != nil { + return e.ContextF() + } + if e.CantContext && e.T != nil { + e.T.Fatalf("Unexpectedly called Context") + } + return nil +} + // Startup ... -func (e *EngineTest) Startup() { +func (e *EngineTest) Startup() error { if e.StartupF != nil { - e.StartupF() - } else if e.CantStartup && e.T != nil { - e.T.Fatalf("Unexpectedly called Startup") + return e.StartupF() + } else if e.CantStartup { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Startup") + } + return errors.New("Unexpectedly called Startup") } + return nil } // Gossip ... -func (e *EngineTest) Gossip() { +func (e *EngineTest) Gossip() error { if e.GossipF != nil { - e.GossipF() - } else if e.CantGossip && e.T != nil { - e.T.Fatalf("Unexpectedly called Gossip") + return e.GossipF() + } else if e.CantGossip { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Gossip") + } + return errors.New("Unexpectedly called Gossip") } + return nil } // Shutdown ... -func (e *EngineTest) Shutdown() { +func (e *EngineTest) Shutdown() error { if e.ShutdownF != nil { - e.ShutdownF() - } else if e.CantShutdown && e.T != nil { - e.T.Fatalf("Unexpectedly called Shutdown") - } -} - -// Context ... -func (e *EngineTest) Context() *snow.Context { - if e.ContextF != nil { - return e.ContextF() - } - if e.CantContext && e.T != nil { - e.T.Fatalf("Unexpectedly called Context") + return e.ShutdownF() + } else if e.CantShutdown { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Shutdown") + } + return errors.New("Unexpectedly called Shutdown") } return nil } // Notify ... -func (e *EngineTest) Notify(msg Message) { +func (e *EngineTest) Notify(msg Message) error { if e.NotifyF != nil { - e.NotifyF(msg) - } else if e.CantNotify && e.T != nil { - e.T.Fatalf("Unexpectedly called Notify") + return e.NotifyF(msg) + } else if e.CantNotify { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Notify") + } + return errors.New("Unexpectedly called Notify") } + return nil } // GetAcceptedFrontier ... -func (e *EngineTest) GetAcceptedFrontier(validatorID ids.ShortID, requestID uint32) { +func (e *EngineTest) GetAcceptedFrontier(validatorID ids.ShortID, requestID uint32) error { if e.GetAcceptedFrontierF != nil { - e.GetAcceptedFrontierF(validatorID, requestID) - } else if e.CantGetAcceptedFrontier && e.T != nil { - e.T.Fatalf("Unexpectedly called GetAcceptedFrontier") + return e.GetAcceptedFrontierF(validatorID, requestID) + } else if e.CantGetAcceptedFrontier { + if e.T != nil { + e.T.Fatalf("Unexpectedly called GetAcceptedFrontier") + } + return errors.New("Unexpectedly called GetAcceptedFrontier") } + return nil } // GetAcceptedFrontierFailed ... -func (e *EngineTest) GetAcceptedFrontierFailed(validatorID ids.ShortID, requestID uint32) { +func (e *EngineTest) GetAcceptedFrontierFailed(validatorID ids.ShortID, requestID uint32) error { if e.GetAcceptedFrontierFailedF != nil { - e.GetAcceptedFrontierFailedF(validatorID, requestID) - } else if e.CantGetAcceptedFrontierFailed && e.T != nil { - e.T.Fatalf("Unexpectedly called GetAcceptedFrontierFailed") + return e.GetAcceptedFrontierFailedF(validatorID, requestID) + } else if e.CantGetAcceptedFrontierFailed { + if e.T != nil { + e.T.Fatalf("Unexpectedly called GetAcceptedFrontierFailed") + } + return errors.New("Unexpectedly called GetAcceptedFrontierFailed") } + return nil } // AcceptedFrontier ... -func (e *EngineTest) AcceptedFrontier(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (e *EngineTest) AcceptedFrontier(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { if e.AcceptedFrontierF != nil { - e.AcceptedFrontierF(validatorID, requestID, containerIDs) - } else if e.CantAcceptedFrontier && e.T != nil { - e.T.Fatalf("Unexpectedly called AcceptedFrontierF") + return e.AcceptedFrontierF(validatorID, requestID, containerIDs) + } else if e.CantAcceptedFrontier { + if e.T != nil { + e.T.Fatalf("Unexpectedly called AcceptedFrontierF") + } + return errors.New("Unexpectedly called AcceptedFrontierF") } + return nil } // GetAccepted ... -func (e *EngineTest) GetAccepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (e *EngineTest) GetAccepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { if e.GetAcceptedF != nil { - e.GetAcceptedF(validatorID, requestID, containerIDs) - } else if e.CantGetAccepted && e.T != nil { - e.T.Fatalf("Unexpectedly called GetAccepted") + return e.GetAcceptedF(validatorID, requestID, containerIDs) + } else if e.CantGetAccepted { + if e.T != nil { + e.T.Fatalf("Unexpectedly called GetAccepted") + } + return errors.New("Unexpectedly called GetAccepted") } + return nil } // GetAcceptedFailed ... -func (e *EngineTest) GetAcceptedFailed(validatorID ids.ShortID, requestID uint32) { +func (e *EngineTest) GetAcceptedFailed(validatorID ids.ShortID, requestID uint32) error { if e.GetAcceptedFailedF != nil { - e.GetAcceptedFailedF(validatorID, requestID) - } else if e.CantGetAcceptedFailed && e.T != nil { - e.T.Fatalf("Unexpectedly called GetAcceptedFailed") + return e.GetAcceptedFailedF(validatorID, requestID) + } else if e.CantGetAcceptedFailed { + if e.T != nil { + e.T.Fatalf("Unexpectedly called GetAcceptedFailed") + } + return errors.New("Unexpectedly called GetAcceptedFailed") } + return nil } // Accepted ... -func (e *EngineTest) Accepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (e *EngineTest) Accepted(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { if e.AcceptedF != nil { - e.AcceptedF(validatorID, requestID, containerIDs) - } else if e.CantAccepted && e.T != nil { - e.T.Fatalf("Unexpectedly called Accepted") + return e.AcceptedF(validatorID, requestID, containerIDs) + } else if e.CantAccepted { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Accepted") + } + return errors.New("Unexpectedly called Accepted") } + return nil } // Get ... -func (e *EngineTest) Get(validatorID ids.ShortID, requestID uint32, containerID ids.ID) { +func (e *EngineTest) Get(validatorID ids.ShortID, requestID uint32, containerID ids.ID) error { if e.GetF != nil { - e.GetF(validatorID, requestID, containerID) - } else if e.CantGet && e.T != nil { - e.T.Fatalf("Unexpectedly called Get") + return e.GetF(validatorID, requestID, containerID) + } else if e.CantGet { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Get") + } + return errors.New("Unexpectedly called Get") } + return nil } // GetFailed ... -func (e *EngineTest) GetFailed(validatorID ids.ShortID, requestID uint32) { +func (e *EngineTest) GetFailed(validatorID ids.ShortID, requestID uint32) error { if e.GetFailedF != nil { - e.GetFailedF(validatorID, requestID) - } else if e.CantGetFailed && e.T != nil { - e.T.Fatalf("Unexpectedly called GetFailed") + return e.GetFailedF(validatorID, requestID) + } else if e.CantGetFailed { + if e.T != nil { + e.T.Fatalf("Unexpectedly called GetFailed") + } + return errors.New("Unexpectedly called GetFailed") } + return nil } // Put ... -func (e *EngineTest) Put(validatorID ids.ShortID, requestID uint32, containerID ids.ID, container []byte) { +func (e *EngineTest) Put(validatorID ids.ShortID, requestID uint32, containerID ids.ID, container []byte) error { if e.PutF != nil { - e.PutF(validatorID, requestID, containerID, container) - } else if e.CantPut && e.T != nil { - e.T.Fatalf("Unexpectedly called Put") + return e.PutF(validatorID, requestID, containerID, container) + } else if e.CantPut { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Put") + } + return errors.New("Unexpectedly called Put") } + return nil } // PushQuery ... -func (e *EngineTest) PushQuery(validatorID ids.ShortID, requestID uint32, containerID ids.ID, container []byte) { +func (e *EngineTest) PushQuery(validatorID ids.ShortID, requestID uint32, containerID ids.ID, container []byte) error { if e.PushQueryF != nil { - e.PushQueryF(validatorID, requestID, containerID, container) - } else if e.CantPushQuery && e.T != nil { - e.T.Fatalf("Unexpectedly called PushQuery") + return e.PushQueryF(validatorID, requestID, containerID, container) + } else if e.CantPushQuery { + if e.T != nil { + e.T.Fatalf("Unexpectedly called PushQuery") + } + return errors.New("Unexpectedly called PushQuery") } + return nil } // PullQuery ... -func (e *EngineTest) PullQuery(validatorID ids.ShortID, requestID uint32, containerID ids.ID) { +func (e *EngineTest) PullQuery(validatorID ids.ShortID, requestID uint32, containerID ids.ID) error { if e.PullQueryF != nil { - e.PullQueryF(validatorID, requestID, containerID) - } else if e.CantPullQuery && e.T != nil { - e.T.Fatalf("Unexpectedly called PullQuery") + return e.PullQueryF(validatorID, requestID, containerID) + } else if e.CantPullQuery { + if e.T != nil { + e.T.Fatalf("Unexpectedly called PullQuery") + } + return errors.New("Unexpectedly called PullQuery") } + return nil } // QueryFailed ... -func (e *EngineTest) QueryFailed(validatorID ids.ShortID, requestID uint32) { +func (e *EngineTest) QueryFailed(validatorID ids.ShortID, requestID uint32) error { if e.QueryFailedF != nil { - e.QueryFailedF(validatorID, requestID) - } else if e.CantQueryFailed && e.T != nil { - e.T.Fatalf("Unexpectedly called QueryFailed") + return e.QueryFailedF(validatorID, requestID) + } else if e.CantQueryFailed { + if e.T != nil { + e.T.Fatalf("Unexpectedly called QueryFailed") + } + return errors.New("Unexpectedly called QueryFailed") } + return nil } // Chits ... -func (e *EngineTest) Chits(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) { +func (e *EngineTest) Chits(validatorID ids.ShortID, requestID uint32, containerIDs ids.Set) error { if e.ChitsF != nil { - e.ChitsF(validatorID, requestID, containerIDs) - } else if e.CantChits && e.T != nil { - e.T.Fatalf("Unexpectedly called Chits") + return e.ChitsF(validatorID, requestID, containerIDs) + } else if e.CantChits { + if e.T != nil { + e.T.Fatalf("Unexpectedly called Chits") + } + return errors.New("Unexpectedly called Chits") } + return nil } diff --git a/snow/engine/common/test_vm.go b/snow/engine/common/test_vm.go index d997c1ef0d7..daf21b79a50 100644 --- a/snow/engine/common/test_vm.go +++ b/snow/engine/common/test_vm.go @@ -22,7 +22,7 @@ type VMTest struct { CantInitialize, CantShutdown, CantCreateHandlers, CantCreateStaticHandlers bool InitializeF func(*snow.Context, database.Database, []byte, chan<- Message, []*Fx) error - ShutdownF func() + ShutdownF func() error CreateHandlersF func() map[string]*HTTPHandler CreateStaticHandlersF func() map[string]*HTTPHandler } @@ -46,12 +46,16 @@ func (vm *VMTest) Initialize(ctx *snow.Context, db database.Database, initState } // Shutdown ... -func (vm *VMTest) Shutdown() { +func (vm *VMTest) Shutdown() error { if vm.ShutdownF != nil { - vm.ShutdownF() - } else if vm.CantShutdown && vm.T != nil { - vm.T.Fatalf("Unexpectedly called Shutdown") + return vm.ShutdownF() + } else if vm.CantShutdown { + if vm.T != nil { + vm.T.Fatalf("Unexpectedly called Shutdown") + } + return errors.New("Unexpectedly called Shutdown") } + return nil } // CreateHandlers ... diff --git a/snow/engine/common/vm.go b/snow/engine/common/vm.go index 15991c50021..55d76c6bd0c 100644 --- a/snow/engine/common/vm.go +++ b/snow/engine/common/vm.go @@ -38,7 +38,7 @@ type VM interface { ) error // Shutdown is called when the node is shutting down. - Shutdown() + Shutdown() error // Creates the HTTP handlers for custom chain network calls. // diff --git a/snow/engine/snowman/bootstrapper.go b/snow/engine/snowman/bootstrapper.go index 5df0f3a40d5..14098283563 100644 --- a/snow/engine/snowman/bootstrapper.go +++ b/snow/engine/snowman/bootstrapper.go @@ -35,11 +35,11 @@ type bootstrapper struct { pending ids.Set finished bool - onFinished func() + onFinished func() error } // Initialize this engine. -func (b *bootstrapper) Initialize(config BootstrapConfig) { +func (b *bootstrapper) Initialize(config BootstrapConfig) error { b.BootstrapConfig = config b.Blocked.SetParser(&parser{ @@ -51,6 +51,7 @@ func (b *bootstrapper) Initialize(config BootstrapConfig) { config.Bootstrapable = b b.Bootstrapper.Initialize(config.Config) + return nil } // CurrentAcceptedFrontier ... @@ -72,7 +73,7 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { } // ForceAccepted ... -func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) { +func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) error { for _, blkID := range acceptedContainerIDs.List() { b.fetch(blkID) } @@ -80,12 +81,13 @@ func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) { if numPending := b.pending.Len(); numPending == 0 { // TODO: This typically indicates bootstrapping has failed, so this // should be handled appropriately - b.finish() + return b.finish() } + return nil } // Put ... -func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkBytes []byte) { +func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkBytes []byte) error { b.BootstrapConfig.Context.Log.Verbo("Put called for blkID %s", blkID) blk, err := b.VM.ParseBlock(blkBytes) @@ -95,7 +97,7 @@ func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkB formatting.DumpBytes{Bytes: blkBytes}) b.GetFailed(vdr, requestID) - return + return nil } if !b.pending.Contains(blk.ID()) { @@ -104,21 +106,22 @@ func (b *bootstrapper) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkB formatting.DumpBytes{Bytes: blkBytes}) b.GetFailed(vdr, requestID) - return + return nil } - b.addBlock(blk) + return b.addBlock(blk) } // GetFailed ... -func (b *bootstrapper) GetFailed(vdr ids.ShortID, requestID uint32) { +func (b *bootstrapper) GetFailed(vdr ids.ShortID, requestID uint32) error { blkID, ok := b.blkReqs.Remove(vdr, requestID) if !ok { b.BootstrapConfig.Context.Log.Debug("GetFailed called without sending the corresponding Get message from %s", vdr) - return + return nil } b.sendRequest(blkID) + return nil } func (b *bootstrapper) fetch(blkID ids.ID) { @@ -152,12 +155,13 @@ func (b *bootstrapper) sendRequest(blkID ids.ID) { b.numPendingRequests.Set(float64(b.pending.Len())) } -func (b *bootstrapper) addBlock(blk snowman.Block) { +func (b *bootstrapper) addBlock(blk snowman.Block) error { b.storeBlock(blk) if numPending := b.pending.Len(); numPending == 0 { - b.finish() + return b.finish() } + return nil } func (b *bootstrapper) storeBlock(blk snowman.Block) { @@ -192,27 +196,33 @@ func (b *bootstrapper) storeBlock(blk snowman.Block) { b.numPendingRequests.Set(float64(numPending)) } -func (b *bootstrapper) finish() { +func (b *bootstrapper) finish() error { if b.finished { - return + return nil } - b.executeAll(b.Blocked, b.numBlocked) + if err := b.executeAll(b.Blocked, b.numBlocked); err != nil { + return err + } // Start consensus - b.onFinished() + if err := b.onFinished(); err != nil { + return err + } b.finished = true if b.Bootstrapped != nil { b.Bootstrapped() } + return nil } -func (b *bootstrapper) executeAll(jobs *queue.Jobs, numBlocked prometheus.Gauge) { +func (b *bootstrapper) executeAll(jobs *queue.Jobs, numBlocked prometheus.Gauge) error { for job, err := jobs.Pop(); err == nil; job, err = jobs.Pop() { numBlocked.Dec() if err := jobs.Execute(job); err != nil { - b.BootstrapConfig.Context.Log.Warn("Error executing: %s", err) + return err } } + return nil } diff --git a/snow/engine/snowman/bootstrapper_test.go b/snow/engine/snowman/bootstrapper_test.go index 3d9752d1372..8b7d3aa8f4c 100644 --- a/snow/engine/snowman/bootstrapper_test.go +++ b/snow/engine/snowman/bootstrapper_test.go @@ -19,7 +19,6 @@ import ( "github.com/ava-labs/gecko/snow/consensus/snowman" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/snow/engine/common/queue" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/router" "github.com/ava-labs/gecko/snow/networking/timeout" "github.com/ava-labs/gecko/snow/validators" @@ -37,7 +36,7 @@ func newConfig(t *testing.T) (BootstrapConfig, ids.ShortID, *common.SenderTest, sender := &common.SenderTest{} vm := &VMTest{} engine := &Transitive{} - handler := &handler.Handler{} + handler := &router.Handler{} router := &router.ChainRouter{} timeouts := &timeout.Manager{} @@ -142,7 +141,7 @@ func TestBootstrapperSingleFrontier(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *reqID, blkID1, blkBytes1) @@ -236,7 +235,7 @@ func TestBootstrapperUnknownByzantineResponse(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *requestID, blkID2, blkBytes2) bs.Put(peerID, *requestID, blkID1, blkBytes1) @@ -336,7 +335,7 @@ func TestBootstrapperDependency(t *testing.T) { blk1.status = choices.Processing finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *requestID, blkID1, blkBytes1) @@ -466,7 +465,7 @@ func TestBootstrapperPartialFetch(t *testing.T) { } sender.CantGet = false - bs.onFinished = func() {} + bs.onFinished = func() error { return nil } bs.ForceAccepted(acceptedIDs) @@ -572,7 +571,7 @@ func TestBootstrapperWrongIDByzantineResponse(t *testing.T) { } finished := new(bool) - bs.onFinished = func() { *finished = true } + bs.onFinished = func() error { *finished = true; return nil } bs.Put(peerID, *requestID, blkID1, blkBytes1) diff --git a/snow/engine/snowman/convincer.go b/snow/engine/snowman/convincer.go index 5eef1e98b22..8f430564f5f 100644 --- a/snow/engine/snowman/convincer.go +++ b/snow/engine/snowman/convincer.go @@ -7,6 +7,7 @@ import ( "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/consensus/snowman" "github.com/ava-labs/gecko/snow/engine/common" + "github.com/ava-labs/gecko/utils/wrappers" ) type convincer struct { @@ -16,6 +17,7 @@ type convincer struct { requestID uint32 abandoned bool deps ids.Set + errs *wrappers.Errs } func (c *convincer) Dependencies() ids.Set { return c.deps } @@ -28,7 +30,7 @@ func (c *convincer) Fulfill(id ids.ID) { func (c *convincer) Abandon(ids.ID) { c.abandoned = true } func (c *convincer) Update() { - if c.abandoned || c.deps.Len() != 0 { + if c.abandoned || c.deps.Len() != 0 || c.errs.Errored() { return } diff --git a/snow/engine/snowman/issuer.go b/snow/engine/snowman/issuer.go index 5e66fd07652..50b1cc0a42f 100644 --- a/snow/engine/snowman/issuer.go +++ b/snow/engine/snowman/issuer.go @@ -36,9 +36,9 @@ func (i *issuer) Abandon(ids.ID) { } func (i *issuer) Update() { - if i.abandoned || i.deps.Len() != 0 { + if i.abandoned || i.deps.Len() != 0 || i.t.errs.Errored() { return } - i.t.deliver(i.blk) + i.t.errs.Add(i.t.deliver(i.blk)) } diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go index c03533b7a82..cbb03400011 100644 --- a/snow/engine/snowman/transitive.go +++ b/snow/engine/snowman/transitive.go @@ -11,6 +11,7 @@ import ( "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/snow/events" "github.com/ava-labs/gecko/utils/formatting" + "github.com/ava-labs/gecko/utils/wrappers" ) // Transitive implements the Engine interface by attempting to fetch all @@ -36,10 +37,13 @@ type Transitive struct { // mark for if the engine has been bootstrapped or not bootstrapped bool + + // errs tracks if an error has occurred in a callback + errs wrappers.Errs } // Initialize implements the Engine interface -func (t *Transitive) Initialize(config Config) { +func (t *Transitive) Initialize(config Config) error { config.Context.Log.Info("Initializing Snowman consensus") t.Config = config @@ -50,17 +54,18 @@ func (t *Transitive) Initialize(config Config) { ) t.onFinished = t.finishBootstrapping - t.bootstrapper.Initialize(config.BootstrapConfig) t.polls.log = config.Context.Log t.polls.numPolls = t.numPolls t.polls.alpha = t.Params.Alpha t.polls.m = make(map[uint32]poll) + + return t.bootstrapper.Initialize(config.BootstrapConfig) } // when bootstrapping is finished, this will be called. This initializes the // consensus engine with the last accepted block. -func (t *Transitive) finishBootstrapping() { +func (t *Transitive) finishBootstrapping() error { // set the bootstrapped mark to switch consensus modes t.bootstrapped = true @@ -74,14 +79,16 @@ func (t *Transitive) finishBootstrapping() { tail, err := t.Config.VM.GetBlock(tailID) if err != nil { t.Config.Context.Log.Error("Failed to get last accepted block due to: %s", err) - return + return err } switch blk := tail.(type) { case OracleBlock: for _, blk := range blk.Options() { // note that deliver will set the VM's preference - t.deliver(blk) + if err := t.deliver(blk); err != nil { + return err + } } default: // if there aren't blocks we need to deliver on startup, we need to set @@ -90,32 +97,34 @@ func (t *Transitive) finishBootstrapping() { } t.Config.Context.Log.Info("Bootstrapping finished with %s as the last accepted block", tailID) + return nil } // Gossip implements the Engine interface -func (t *Transitive) Gossip() { +func (t *Transitive) Gossip() error { blkID := t.Config.VM.LastAccepted() blk, err := t.Config.VM.GetBlock(blkID) if err != nil { t.Config.Context.Log.Warn("Dropping gossip request as %s couldn't be loaded due to %s", blkID, err) - return + return nil } t.Config.Context.Log.Debug("Gossiping %s as accepted to the network", blkID) t.Config.Sender.Gossip(blkID, blk.Bytes()) + return nil } // Shutdown implements the Engine interface -func (t *Transitive) Shutdown() { +func (t *Transitive) Shutdown() error { t.Config.Context.Log.Info("Shutting down Snowman consensus") - t.Config.VM.Shutdown() + return t.Config.VM.Shutdown() } // Context implements the Engine interface func (t *Transitive) Context() *snow.Context { return t.Config.Context } // Get implements the Engine interface -func (t *Transitive) Get(vdr ids.ShortID, requestID uint32, blkID ids.ID) { +func (t *Transitive) Get(vdr ids.ShortID, requestID uint32, blkID ids.ID) error { blk, err := t.Config.VM.GetBlock(blkID) if err != nil { // If we failed to get the block, that means either an unexpected error @@ -124,22 +133,22 @@ func (t *Transitive) Get(vdr ids.ShortID, requestID uint32, blkID ids.ID) { t.Config.Context.Log.Warn("Get called for blockID %s errored with %s", blkID, err) - return + return nil } // Respond to the validator with the fetched block and the same requestID. t.Config.Sender.Put(vdr, requestID, blkID, blk.Bytes()) + return nil } // Put implements the Engine interface -func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkBytes []byte) { +func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkBytes []byte) error { t.Config.Context.Log.Verbo("Put called for blockID %s", blkID) // if the engine hasn't been bootstrapped, forward the request to the // bootstrapper if !t.bootstrapped { - t.bootstrapper.Put(vdr, requestID, blkID, blkBytes) - return + return t.bootstrapper.Put(vdr, requestID, blkID, blkBytes) } blk, err := t.Config.VM.ParseBlock(blkBytes) @@ -151,8 +160,7 @@ func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkByt // because GetFailed doesn't utilize the assumption that we actually // sent a Get message, we can safely call GetFailed here to potentially // abandon the request. - t.GetFailed(vdr, requestID) - return + return t.GetFailed(vdr, requestID) } // insert the block into consensus. If the block has already been issued, @@ -160,16 +168,16 @@ func (t *Transitive) Put(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkByt // receive requests to fill the ancestry. dependencies that have already // been fetched, but with missing dependencies themselves won't be requested // from the vdr. - t.insertFrom(vdr, blk) + _, err = t.insertFrom(vdr, blk) + return err } // GetFailed implements the Engine interface -func (t *Transitive) GetFailed(vdr ids.ShortID, requestID uint32) { +func (t *Transitive) GetFailed(vdr ids.ShortID, requestID uint32) error { // if the engine hasn't been bootstrapped, forward the request to the // bootstrapper if !t.bootstrapped { - t.bootstrapper.GetFailed(vdr, requestID) - return + return t.bootstrapper.GetFailed(vdr, requestID) } // we don't use the assumption that this function is called after a failed @@ -179,22 +187,23 @@ func (t *Transitive) GetFailed(vdr ids.ShortID, requestID uint32) { if !ok { t.Config.Context.Log.Warn("GetFailed called without sending the corresponding Get message from %s", vdr) - return + return nil } // because the get request was dropped, we no longer are expected blkID to // be issued. t.blocked.Abandon(blkID) + return t.errs.Err } // PullQuery implements the Engine interface -func (t *Transitive) PullQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID) { +func (t *Transitive) PullQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID) error { // if the engine hasn't been bootstrapped, we aren't ready to respond to // queries if !t.bootstrapped { t.Config.Context.Log.Debug("Dropping PullQuery for %s due to bootstrapping", blkID) - return + return nil } c := &convincer{ @@ -202,24 +211,31 @@ func (t *Transitive) PullQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID) sender: t.Config.Sender, vdr: vdr, requestID: requestID, + errs: &t.errs, + } + + added, err := t.reinsertFrom(vdr, blkID) + if err != nil { + return err } // if we aren't able to have issued this block, then it is a dependency for // this reply - if !t.reinsertFrom(vdr, blkID) { + if !added { c.deps.Add(blkID) } t.blocked.Register(c) + return t.errs.Err } // PushQuery implements the Engine interface -func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkBytes []byte) { +func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID, blkBytes []byte) error { // if the engine hasn't been bootstrapped, we aren't ready to respond to // queries if !t.bootstrapped { t.Config.Context.Log.Debug("Dropping PushQuery for %s due to bootstrapping", blkID) - return + return nil } blk, err := t.Config.VM.ParseBlock(blkBytes) @@ -228,7 +244,7 @@ func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID, t.Config.Context.Log.Warn("ParseBlock failed due to %s for block:\n%s", err, formatting.DumpBytes{Bytes: blkBytes}) - return + return nil } // insert the block into consensus. If the block has already been issued, @@ -236,18 +252,20 @@ func (t *Transitive) PushQuery(vdr ids.ShortID, requestID uint32, blkID ids.ID, // receive requests to fill the ancestry. dependencies that have already // been fetched, but with missing dependencies themselves won't be requested // from the vdr. - t.insertFrom(vdr, blk) + if _, err := t.insertFrom(vdr, blk); err != nil { + return err + } // register the chit request - t.PullQuery(vdr, requestID, blk.ID()) + return t.PullQuery(vdr, requestID, blk.ID()) } // Chits implements the Engine interface -func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) { +func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) error { // if the engine hasn't been bootstrapped, we shouldn't be receiving chits if !t.bootstrapped { t.Config.Context.Log.Debug("Dropping Chits due to bootstrapping") - return + return nil } // Since this is snowman, there should only be one ID in the vote set @@ -260,8 +278,7 @@ func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) { // because QueryFailed doesn't utilize the assumption that we actually // sent a Query message, we can safely call QueryFailed here to // potentially abandon the request. - t.QueryFailed(vdr, requestID) - return + return t.QueryFailed(vdr, requestID) } vote := votes.List()[0] @@ -274,21 +291,27 @@ func (t *Transitive) Chits(vdr ids.ShortID, requestID uint32, votes ids.Set) { response: vote, } + added, err := t.reinsertFrom(vdr, vote) + if err != nil { + return err + } + // if we aren't able to have issued the vote's block, then it is a // dependency for applying the vote - if !t.reinsertFrom(vdr, vote) { + if !added { v.deps.Add(vote) } t.blocked.Register(v) + return t.errs.Err } // QueryFailed implements the Engine interface -func (t *Transitive) QueryFailed(vdr ids.ShortID, requestID uint32) { +func (t *Transitive) QueryFailed(vdr ids.ShortID, requestID uint32) error { // if the engine hasn't been bootstrapped, we won't have sent a query if !t.bootstrapped { t.Config.Context.Log.Warn("Dropping QueryFailed due to bootstrapping") - return + return nil } t.blocked.Register(&voter{ @@ -296,14 +319,15 @@ func (t *Transitive) QueryFailed(vdr ids.ShortID, requestID uint32) { vdr: vdr, requestID: requestID, }) + return t.errs.Err } // Notify implements the Engine interface -func (t *Transitive) Notify(msg common.Message) { +func (t *Transitive) Notify(msg common.Message) error { // if the engine hasn't been bootstrapped, we shouldn't issuing blocks if !t.bootstrapped { t.Config.Context.Log.Warn("Dropping Notify due to bootstrapping") - return + return nil } t.Config.Context.Log.Verbo("Snowman engine notified of %s from the vm", msg) @@ -313,7 +337,7 @@ func (t *Transitive) Notify(msg common.Message) { blk, err := t.Config.VM.BuildBlock() if err != nil { t.Config.Context.Log.Verbo("VM.BuildBlock errored with %s", err) - return + return nil } // a newly created block is expected to be processing. If this check @@ -330,8 +354,13 @@ func (t *Transitive) Notify(msg common.Message) { t.Config.Context.Log.Warn("Built block with parent: %s, expected %s", parentID, pref) } + added, err := t.insertAll(blk) + if err != nil { + return err + } + // inserting the block shouldn't have any missing dependencies - if t.insertAll(blk) { + if added { t.Config.Context.Log.Verbo("Successfully issued new block from the VM") } else { t.Config.Context.Log.Warn("VM.BuildBlock returned a block that is pending for ancestors") @@ -339,6 +368,7 @@ func (t *Transitive) Notify(msg common.Message) { default: t.Config.Context.Log.Warn("Unexpected message from the VM: %s", msg) } + return nil } func (t *Transitive) repoll() { @@ -356,11 +386,11 @@ func (t *Transitive) repoll() { // added, to consensus. This is useful to check the local DB before requesting a // block in case we have the block for some reason. If the block or a dependency // is missing, the validator will be sent a Get message. -func (t *Transitive) reinsertFrom(vdr ids.ShortID, blkID ids.ID) bool { +func (t *Transitive) reinsertFrom(vdr ids.ShortID, blkID ids.ID) (bool, error) { blk, err := t.Config.VM.GetBlock(blkID) if err != nil { t.sendRequest(vdr, blkID) - return false + return false, nil } return t.insertFrom(vdr, blk) } @@ -370,12 +400,14 @@ func (t *Transitive) reinsertFrom(vdr ids.ShortID, blkID ids.ID) bool { // This is useful to check the local DB before requesting a block in case we // have the block for some reason. If a dependency is missing, the validator // will be sent a Get message. -func (t *Transitive) insertFrom(vdr ids.ShortID, blk snowman.Block) bool { +func (t *Transitive) insertFrom(vdr ids.ShortID, blk snowman.Block) (bool, error) { blkID := blk.ID() // if the block has been issued, we don't need to insert it. if the block is // already pending, we shouldn't attempt to insert it again yet for !t.Consensus.Issued(blk) && !t.pending.Contains(blkID) { - t.insert(blk) + if err := t.insert(blk); err != nil { + return false, err + } blk = blk.Parent() blkID = blk.ID() @@ -384,10 +416,10 @@ func (t *Transitive) insertFrom(vdr ids.ShortID, blk snowman.Block) bool { // newly inserted block if !blk.Status().Fetched() { t.sendRequest(vdr, blkID) - return false + return false, nil } } - return t.Consensus.Issued(blk) + return t.Consensus.Issued(blk), nil } // insertAll attempts to issue the branch ending with a block to consensus. @@ -395,10 +427,12 @@ func (t *Transitive) insertFrom(vdr ids.ShortID, blk snowman.Block) bool { // This is useful to check the local DB before requesting a block in case we // have the block for some reason. If a dependency is missing and the dependency // hasn't been requested, the issuance will be abandoned. -func (t *Transitive) insertAll(blk snowman.Block) bool { +func (t *Transitive) insertAll(blk snowman.Block) (bool, error) { blkID := blk.ID() for blk.Status().Fetched() && !t.Consensus.Issued(blk) && !t.pending.Contains(blkID) { - t.insert(blk) + if err := t.insert(blk); err != nil { + return false, err + } blk = blk.Parent() blkID = blk.ID() @@ -406,25 +440,25 @@ func (t *Transitive) insertAll(blk snowman.Block) bool { // if issuance the block was successful, this is the happy path if t.Consensus.Issued(blk) { - return true + return true, nil } // if this branch is waiting on a block that we supposedly have a source of, // we can just wait for that request to succeed or fail if t.blkReqs.Contains(blkID) { - return false + return false, nil } // if we have no reason to expect that this block will be inserted, we // should abandon the block to avoid a memory leak t.blocked.Abandon(blkID) - return false + return false, t.errs.Err } // attempt to insert the block to consensus. If the block's parent hasn't been // issued, the insertion will block until the parent's issuance is abandoned or // fulfilled -func (t *Transitive) insert(blk snowman.Block) { +func (t *Transitive) insert(blk snowman.Block) error { blkID := blk.ID() // mark that the block has been fetched but is pending @@ -451,6 +485,7 @@ func (t *Transitive) insert(blk snowman.Block) { // Tracks performance statistics t.numBlkRequests.Set(float64(t.blkReqs.Len())) t.numBlockedBlk.Set(float64(t.pending.Len())) + return t.errs.Err } func (t *Transitive) sendRequest(vdr ids.ShortID, blkID ids.ID) { @@ -519,9 +554,9 @@ func (t *Transitive) pushSample(blk snowman.Block) { return } -func (t *Transitive) deliver(blk snowman.Block) { +func (t *Transitive) deliver(blk snowman.Block) error { if t.Consensus.Issued(blk) { - return + return nil } // we are adding the block to consensus, so it is no longer pending @@ -534,7 +569,7 @@ func (t *Transitive) deliver(blk snowman.Block) { // if verify fails, then all decedents are also invalid t.blocked.Abandon(blkID) t.numBlockedBlk.Set(float64(t.pending.Len())) // Tracks performance statistics - return + return t.errs.Err } t.Config.Context.Log.Verbo("Adding block to consensus: %s", blkID) @@ -583,4 +618,5 @@ func (t *Transitive) deliver(blk snowman.Block) { // Tracks performance statistics t.numBlkRequests.Set(float64(t.blkReqs.Len())) t.numBlockedBlk.Set(float64(t.pending.Len())) + return t.errs.Err } diff --git a/snow/engine/snowman/transitive_test.go b/snow/engine/snowman/transitive_test.go index b68d08e8dae..a3db1d6482c 100644 --- a/snow/engine/snowman/transitive_test.go +++ b/snow/engine/snowman/transitive_test.go @@ -76,7 +76,7 @@ func setup(t *testing.T) (validators.Validator, validators.Set, *common.SenderTe func TestEngineShutdown(t *testing.T) { _, _, _, vm, transitive, _ := setup(t) vmShutdownCalled := false - vm.ShutdownF = func() { vmShutdownCalled = true } + vm.ShutdownF = func() error { vmShutdownCalled = true; return nil } vm.CantShutdown = false transitive.Shutdown() if !vmShutdownCalled { diff --git a/snow/engine/snowman/voter.go b/snow/engine/snowman/voter.go index 2c8b20954e2..25f9ab0079a 100644 --- a/snow/engine/snowman/voter.go +++ b/snow/engine/snowman/voter.go @@ -25,7 +25,7 @@ func (v *voter) Fulfill(id ids.ID) { func (v *voter) Abandon(id ids.ID) { v.Fulfill(id) } func (v *voter) Update() { - if v.deps.Len() != 0 { + if v.deps.Len() != 0 || v.t.errs.Errored() { return } @@ -46,7 +46,10 @@ func (v *voter) Update() { results = v.bubbleVotes(results) v.t.Config.Context.Log.Verbo("Finishing poll [%d] with:\n%s", v.requestID, &results) - v.t.Consensus.RecordPoll(results) + if err := v.t.Consensus.RecordPoll(results); err != nil { + v.t.errs.Add(err) + return + } v.t.Config.VM.SetPreference(v.t.Consensus.Preference()) diff --git a/snow/networking/handler/handler.go b/snow/networking/router/handler.go similarity index 78% rename from snow/networking/handler/handler.go rename to snow/networking/router/handler.go index 6f46909bccb..b2ca7103604 100644 --- a/snow/networking/handler/handler.go +++ b/snow/networking/router/handler.go @@ -1,7 +1,7 @@ // (c) 2019-2020, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package handler +package router import ( "sync" @@ -18,6 +18,8 @@ type Handler struct { wg sync.WaitGroup engine common.Engine msgChan <-chan common.Message + + toClose func() } // Initialize this consensus handler @@ -35,24 +37,40 @@ func (h *Handler) Context() *snow.Context { return h.engine.Context() } // Dispatch waits for incoming messages from the network // and, when they arrive, sends them to the consensus engine func (h *Handler) Dispatch() { + log := h.Context().Log defer h.wg.Done() + closing := false for { select { - case msg := <-h.msgs: - if !h.dispatchMsg(msg) { + case msg, ok := <-h.msgs: + if !ok { return } + if closing { + log.Debug("dropping message due to closing:\n%s", msg) + continue + } + if !h.dispatchMsg(msg) { + closing = true + } case msg := <-h.msgChan: + if closing { + log.Debug("dropping internal message due to closing:\n%s", msg) + continue + } if !h.dispatchMsg(message{messageType: notifyMsg, notification: msg}) { - return + closing = true } } + if closing && h.toClose != nil { + go h.toClose() + } } } // Dispatch a message to the consensus engine. -// Returns false iff this consensus handler (and its associated engine) should shutdown +// Returns true iff this consensus handler (and its associated engine) should shutdown // (due to receipt of a shutdown message) func (h *Handler) dispatchMsg(msg message) bool { ctx := h.engine.Context() @@ -61,43 +79,50 @@ func (h *Handler) dispatchMsg(msg message) bool { defer ctx.Lock.Unlock() ctx.Log.Verbo("Forwarding message to consensus: %s", msg) - + var ( + err error + done bool + ) switch msg.messageType { case getAcceptedFrontierMsg: - h.engine.GetAcceptedFrontier(msg.validatorID, msg.requestID) + err = h.engine.GetAcceptedFrontier(msg.validatorID, msg.requestID) case acceptedFrontierMsg: - h.engine.AcceptedFrontier(msg.validatorID, msg.requestID, msg.containerIDs) + err = h.engine.AcceptedFrontier(msg.validatorID, msg.requestID, msg.containerIDs) case getAcceptedFrontierFailedMsg: - h.engine.GetAcceptedFrontierFailed(msg.validatorID, msg.requestID) + err = h.engine.GetAcceptedFrontierFailed(msg.validatorID, msg.requestID) case getAcceptedMsg: - h.engine.GetAccepted(msg.validatorID, msg.requestID, msg.containerIDs) + err = h.engine.GetAccepted(msg.validatorID, msg.requestID, msg.containerIDs) case acceptedMsg: - h.engine.Accepted(msg.validatorID, msg.requestID, msg.containerIDs) + err = h.engine.Accepted(msg.validatorID, msg.requestID, msg.containerIDs) case getAcceptedFailedMsg: - h.engine.GetAcceptedFailed(msg.validatorID, msg.requestID) + err = h.engine.GetAcceptedFailed(msg.validatorID, msg.requestID) case getMsg: - h.engine.Get(msg.validatorID, msg.requestID, msg.containerID) + err = h.engine.Get(msg.validatorID, msg.requestID, msg.containerID) case getFailedMsg: - h.engine.GetFailed(msg.validatorID, msg.requestID) + err = h.engine.GetFailed(msg.validatorID, msg.requestID) case putMsg: - h.engine.Put(msg.validatorID, msg.requestID, msg.containerID, msg.container) + err = h.engine.Put(msg.validatorID, msg.requestID, msg.containerID, msg.container) case pushQueryMsg: - h.engine.PushQuery(msg.validatorID, msg.requestID, msg.containerID, msg.container) + err = h.engine.PushQuery(msg.validatorID, msg.requestID, msg.containerID, msg.container) case pullQueryMsg: - h.engine.PullQuery(msg.validatorID, msg.requestID, msg.containerID) + err = h.engine.PullQuery(msg.validatorID, msg.requestID, msg.containerID) case queryFailedMsg: - h.engine.QueryFailed(msg.validatorID, msg.requestID) + err = h.engine.QueryFailed(msg.validatorID, msg.requestID) case chitsMsg: - h.engine.Chits(msg.validatorID, msg.requestID, msg.containerIDs) + err = h.engine.Chits(msg.validatorID, msg.requestID, msg.containerIDs) case notifyMsg: - h.engine.Notify(msg.notification) + err = h.engine.Notify(msg.notification) case gossipMsg: - h.engine.Gossip() + err = h.engine.Gossip() case shutdownMsg: - h.engine.Shutdown() - return false + err = h.engine.Shutdown() + done = true + } + + if err != nil { + ctx.Log.Error("fatal error occurred on chain %s, forcing a shutdown due to %s", err) } - return true + return done || err != nil } // GetAcceptedFrontier passes a GetAcceptedFrontier message received from the @@ -237,7 +262,7 @@ func (h *Handler) QueryFailed(validatorID ids.ShortID, requestID uint32) { func (h *Handler) Gossip() { h.msgs <- message{messageType: gossipMsg} } // Shutdown shuts down the dispatcher -func (h *Handler) Shutdown() { h.msgs <- message{messageType: shutdownMsg}; h.wg.Wait() } +func (h *Handler) Shutdown() { h.msgs <- message{messageType: shutdownMsg} } // Notify ... func (h *Handler) Notify(msg common.Message) { diff --git a/snow/networking/handler/message.go b/snow/networking/router/message.go similarity index 99% rename from snow/networking/handler/message.go rename to snow/networking/router/message.go index f5394f0f26d..9a0e208c797 100644 --- a/snow/networking/handler/message.go +++ b/snow/networking/router/message.go @@ -1,7 +1,7 @@ // (c) 2019-2020, Ava Labs, Inc. All rights reserved. // See the file LICENSE for licensing terms. -package handler +package router import ( "fmt" diff --git a/snow/networking/router/router.go b/snow/networking/router/router.go index d007f543201..93c7bcc2ff1 100644 --- a/snow/networking/router/router.go +++ b/snow/networking/router/router.go @@ -7,7 +7,6 @@ import ( "time" "github.com/ava-labs/gecko/ids" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/timeout" "github.com/ava-labs/gecko/utils/logging" ) @@ -18,7 +17,7 @@ type Router interface { ExternalRouter InternalRouter - AddChain(chain *handler.Handler) + AddChain(chain *Handler) RemoveChain(chainID ids.ID) Shutdown() Initialize(log logging.Logger, timeouts *timeout.Manager, gossipFrequency time.Duration) diff --git a/snow/networking/router/subnet_router.go b/snow/networking/router/subnet_router.go index ac95141bc82..66f1341b0ff 100644 --- a/snow/networking/router/subnet_router.go +++ b/snow/networking/router/subnet_router.go @@ -8,7 +8,6 @@ import ( "time" "github.com/ava-labs/gecko/ids" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/timeout" "github.com/ava-labs/gecko/utils/logging" "github.com/ava-labs/gecko/utils/timer" @@ -21,7 +20,7 @@ import ( type ChainRouter struct { log logging.Logger lock sync.RWMutex - chains map[[32]byte]*handler.Handler + chains map[[32]byte]*Handler timeouts *timeout.Manager gossiper *timer.Repeater } @@ -36,7 +35,7 @@ type ChainRouter struct { // notifying the engine it should gossip it's accepted set. func (sr *ChainRouter) Initialize(log logging.Logger, timeouts *timeout.Manager, gossipFrequency time.Duration) { sr.log = log - sr.chains = make(map[[32]byte]*handler.Handler) + sr.chains = make(map[[32]byte]*Handler) sr.timeouts = timeouts sr.gossiper = timer.NewRepeater(sr.Gossip, gossipFrequency) @@ -45,12 +44,13 @@ func (sr *ChainRouter) Initialize(log logging.Logger, timeouts *timeout.Manager, // AddChain registers the specified chain so that incoming // messages can be routed to it -func (sr *ChainRouter) AddChain(chain *handler.Handler) { +func (sr *ChainRouter) AddChain(chain *Handler) { sr.lock.Lock() defer sr.lock.Unlock() chainID := chain.Context().ChainID sr.log.Debug("registering chain %s with chain router", chainID) + chain.toClose = func() { sr.RemoveChain(chainID) } sr.chains[chainID.Key()] = chain } @@ -62,6 +62,8 @@ func (sr *ChainRouter) RemoveChain(chainID ids.ID) { if chain, exists := sr.chains[chainID.Key()]; exists { chain.Shutdown() + close(chain.msgs) + chain.wg.Wait() delete(sr.chains, chainID.Key()) } else { sr.log.Debug("message referenced a chain, %s, this node doesn't validate", chainID) @@ -256,16 +258,16 @@ func (sr *ChainRouter) QueryFailed(validatorID ids.ShortID, chainID ids.ID, requ // Shutdown shuts down this router func (sr *ChainRouter) Shutdown() { - sr.lock.RLock() - defer sr.lock.RUnlock() - - sr.shutdown() -} - -func (sr *ChainRouter) shutdown() { + sr.lock.Lock() for _, chain := range sr.chains { chain.Shutdown() + close(chain.msgs) + } + for _, chain := range sr.chains { + chain.wg.Wait() } + sr.chains = map[[32]byte]*Handler{} + sr.lock.Unlock() sr.gossiper.Stop() } @@ -274,10 +276,6 @@ func (sr *ChainRouter) Gossip() { sr.lock.RLock() defer sr.lock.RUnlock() - sr.gossip() -} - -func (sr *ChainRouter) gossip() { for _, chain := range sr.chains { chain.Gossip() } diff --git a/snow/networking/sender/sender_test.go b/snow/networking/sender/sender_test.go index 5a0ef83fc72..a8361d5f7fc 100644 --- a/snow/networking/sender/sender_test.go +++ b/snow/networking/sender/sender_test.go @@ -12,7 +12,6 @@ import ( "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow" "github.com/ava-labs/gecko/snow/engine/common" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/router" "github.com/ava-labs/gecko/snow/networking/timeout" "github.com/ava-labs/gecko/utils/logging" @@ -37,11 +36,11 @@ func TestTimeout(t *testing.T) { tm.Initialize(time.Millisecond) go tm.Dispatch() - router := router.ChainRouter{} - router.Initialize(logging.NoLog{}, &tm, time.Hour) + chainRouter := router.ChainRouter{} + chainRouter.Initialize(logging.NoLog{}, &tm, time.Hour) sender := Sender{} - sender.Initialize(snow.DefaultContextTest(), &ExternalSenderTest{}, &router, &tm) + sender.Initialize(snow.DefaultContextTest(), &ExternalSenderTest{}, &chainRouter, &tm) engine := common.EngineTest{T: t} engine.Default(true) @@ -52,16 +51,17 @@ func TestTimeout(t *testing.T) { wg.Add(2) failedVDRs := ids.ShortSet{} - engine.QueryFailedF = func(validatorID ids.ShortID, _ uint32) { + engine.QueryFailedF = func(validatorID ids.ShortID, _ uint32) error { failedVDRs.Add(validatorID) wg.Done() + return nil } - handler := handler.Handler{} + handler := router.Handler{} handler.Initialize(&engine, nil, 1) go handler.Dispatch() - router.AddChain(&handler) + chainRouter.AddChain(&handler) vdrIDs := ids.ShortSet{} vdrIDs.Add(ids.NewShortID([20]byte{255})) diff --git a/vms/avm/vm.go b/vms/avm/vm.go index 180f4ae8185..3c965272a54 100644 --- a/vms/avm/vm.go +++ b/vms/avm/vm.go @@ -199,9 +199,9 @@ func (vm *VM) Initialize( } // Shutdown implements the avalanche.DAGVM interface -func (vm *VM) Shutdown() { +func (vm *VM) Shutdown() error { if vm.timer == nil { - return + return nil } // There is a potential deadlock if the timer is about to execute a timeout. @@ -210,9 +210,7 @@ func (vm *VM) Shutdown() { vm.timer.Stop() vm.ctx.Lock.Lock() - if err := vm.baseDB.Close(); err != nil { - vm.ctx.Log.Error("Closing the database failed with %s", err) - } + return vm.baseDB.Close() } // CreateHandlers implements the avalanche.DAGVM interface diff --git a/vms/components/core/snowman_vm.go b/vms/components/core/snowman_vm.go index e67dc662ab5..eb3787d892c 100644 --- a/vms/components/core/snowman_vm.go +++ b/vms/components/core/snowman_vm.go @@ -82,14 +82,21 @@ func (svm *SnowmanVM) GetBlock(ID ids.ID) (snowman.Block, error) { } // Shutdown this vm -func (svm *SnowmanVM) Shutdown() { +func (svm *SnowmanVM) Shutdown() error { if svm.DB == nil { - return + return nil } - svm.DB.Commit() // Flush DB - svm.DB.GetDatabase().Close() // close underlying database - svm.DB.Close() // close versionDB + // flush DB + if err := svm.DB.Commit(); err != nil { + return err + } + + // close underlying database + if err := svm.DB.GetDatabase().Close(); err != nil { + return err + } + return svm.DB.Close() // close versionDB } // DBInitialized returns true iff [svm]'s database has values in it already diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index e13e527e723..9416a195c7a 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -400,9 +400,9 @@ func (vm *VM) createChain(tx *CreateChainTx) { } // Shutdown this blockchain -func (vm *VM) Shutdown() { +func (vm *VM) Shutdown() error { if vm.timer == nil { - return + return nil } // There is a potential deadlock if the timer is about to execute a timeout. @@ -411,9 +411,7 @@ func (vm *VM) Shutdown() { vm.timer.Stop() vm.Ctx.Lock.Lock() - if err := vm.DB.Close(); err != nil { - vm.Ctx.Log.Error("Closing the database failed with %s", err) - } + return vm.DB.Close() } // BuildBlock builds a block to be added to consensus diff --git a/vms/platformvm/vm_test.go b/vms/platformvm/vm_test.go index 8c0a7f9ccc8..10b08a07875 100644 --- a/vms/platformvm/vm_test.go +++ b/vms/platformvm/vm_test.go @@ -22,7 +22,6 @@ import ( "github.com/ava-labs/gecko/snow/consensus/snowball" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/snow/engine/common/queue" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/router" "github.com/ava-labs/gecko/snow/networking/sender" "github.com/ava-labs/gecko/snow/networking/timeout" @@ -1560,8 +1559,8 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { timeoutManager.Initialize(2 * time.Second) go timeoutManager.Dispatch() - router := &router.ChainRouter{} - router.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) + chainRouter := &router.ChainRouter{} + chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) externalSender := &sender.ExternalSenderTest{T: t} externalSender.Default(true) @@ -1569,7 +1568,7 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { // Passes messages from the consensus engine to the network sender := sender.Sender{} - sender.Initialize(ctx, externalSender, router, &timeoutManager) + sender.Initialize(ctx, externalSender, chainRouter, &timeoutManager) // The engine handles consensus engine := smeng.Transitive{} @@ -1597,11 +1596,11 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { }) // Asynchronously passes messages from the network to the consensus engine - handler := &handler.Handler{} + handler := &router.Handler{} handler.Initialize(&engine, msgChan, 1000) // Allow incoming messages to be routed to the new chain - router.AddChain(handler) + chainRouter.AddChain(handler) go ctx.Log.RecoverAndPanic(handler.Dispatch) reqID := new(uint32) @@ -1646,7 +1645,7 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { } ctx.Lock.Unlock() - router.Shutdown() + chainRouter.Shutdown() } func TestUnverifiedParent(t *testing.T) { diff --git a/vms/rpcchainvm/vm_client.go b/vms/rpcchainvm/vm_client.go index 8ab5229c9b8..b29d5b1abf2 100644 --- a/vms/rpcchainvm/vm_client.go +++ b/vms/rpcchainvm/vm_client.go @@ -130,26 +130,31 @@ func (vm *VMClient) startMessengerServer(opts []grpc.ServerOption) *grpc.Server } // Shutdown ... -func (vm *VMClient) Shutdown() { +func (vm *VMClient) Shutdown() error { vm.lock.Lock() defer vm.lock.Unlock() if vm.closed { - return + return nil } vm.closed = true - vm.client.Shutdown(context.Background(), &vmproto.ShutdownRequest{}) + if _, err := vm.client.Shutdown(context.Background(), &vmproto.ShutdownRequest{}); err != nil { + return err + } for _, server := range vm.servers { server.Stop() } for _, conn := range vm.conns { - conn.Close() + if err := conn.Close(); err != nil { + return err + } } vm.proc.Kill() + return nil } // CreateHandlers ... diff --git a/vms/rpcchainvm/vm_server.go b/vms/rpcchainvm/vm_server.go index 1a2470458d3..b372e79f215 100644 --- a/vms/rpcchainvm/vm_server.go +++ b/vms/rpcchainvm/vm_server.go @@ -95,10 +95,10 @@ func (vm *VMServer) Shutdown(_ context.Context, _ *vmproto.ShutdownRequest) (*vm vm.closed = true - vm.vm.Shutdown() + errs := wrappers.Errs{} + errs.Add(vm.vm.Shutdown()) close(vm.toEngine) - errs := wrappers.Errs{} for _, conn := range vm.conns { errs.Add(conn.Close()) } diff --git a/vms/spchainvm/consensus_benchmark_test.go b/vms/spchainvm/consensus_benchmark_test.go index 8cc72d675e7..b304320e02a 100644 --- a/vms/spchainvm/consensus_benchmark_test.go +++ b/vms/spchainvm/consensus_benchmark_test.go @@ -15,7 +15,6 @@ import ( "github.com/ava-labs/gecko/snow/consensus/snowball" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/snow/engine/common/queue" - "github.com/ava-labs/gecko/snow/networking/handler" "github.com/ava-labs/gecko/snow/networking/router" "github.com/ava-labs/gecko/snow/networking/sender" "github.com/ava-labs/gecko/snow/networking/timeout" @@ -61,8 +60,8 @@ func ConsensusLeader(numBlocks, numTxsPerBlock int, b *testing.B) { timeoutManager.Initialize(2 * time.Second) go timeoutManager.Dispatch() - router := &router.ChainRouter{} - router.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) + chainRouter := &router.ChainRouter{} + chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) // Initialize the VM vm := &VM{} @@ -77,7 +76,7 @@ func ConsensusLeader(numBlocks, numTxsPerBlock int, b *testing.B) { // Passes messages from the consensus engine to the network sender := sender.Sender{} - sender.Initialize(ctx, externalSender, router, &timeoutManager) + sender.Initialize(ctx, externalSender, chainRouter, &timeoutManager) // The engine handles consensus engine := smeng.Transitive{} @@ -105,11 +104,11 @@ func ConsensusLeader(numBlocks, numTxsPerBlock int, b *testing.B) { }) // Asynchronously passes messages from the network to the consensus engine - handler := &handler.Handler{} + handler := &router.Handler{} handler.Initialize(&engine, msgChan, 1000) // Allow incoming messages to be routed to the new chain - router.AddChain(handler) + chainRouter.AddChain(handler) go ctx.Log.RecoverAndPanic(handler.Dispatch) engine.Startup() @@ -189,8 +188,8 @@ func ConsensusFollower(numBlocks, numTxsPerBlock int, b *testing.B) { timeoutManager.Initialize(2 * time.Second) go timeoutManager.Dispatch() - router := &router.ChainRouter{} - router.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) + chainRouter := &router.ChainRouter{} + chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) wg := sync.WaitGroup{} wg.Add(numBlocks) @@ -210,7 +209,7 @@ func ConsensusFollower(numBlocks, numTxsPerBlock int, b *testing.B) { // Passes messages from the consensus engine to the network sender := sender.Sender{} - sender.Initialize(ctx, externalSender, router, &timeoutManager) + sender.Initialize(ctx, externalSender, chainRouter, &timeoutManager) // The engine handles consensus engine := smeng.Transitive{} @@ -238,11 +237,11 @@ func ConsensusFollower(numBlocks, numTxsPerBlock int, b *testing.B) { }) // Asynchronously passes messages from the network to the consensus engine - handler := &handler.Handler{} + handler := &router.Handler{} handler.Initialize(&engine, msgChan, 1000) // Allow incoming messages to be routed to the new chain - router.AddChain(handler) + chainRouter.AddChain(handler) go ctx.Log.RecoverAndPanic(handler.Dispatch) engine.Startup() @@ -250,7 +249,7 @@ func ConsensusFollower(numBlocks, numTxsPerBlock int, b *testing.B) { b.StartTimer() for _, block := range blocks { - router.Put(ctx.NodeID, ctx.ChainID, 0, block.ID(), block.Bytes()) + chainRouter.Put(ctx.NodeID, ctx.ChainID, 0, block.ID(), block.Bytes()) } wg.Wait() b.StopTimer() diff --git a/vms/spchainvm/vm.go b/vms/spchainvm/vm.go index 8ed6309b94d..27dc0204a72 100644 --- a/vms/spchainvm/vm.go +++ b/vms/spchainvm/vm.go @@ -117,9 +117,9 @@ func (vm *VM) Initialize( } // Shutdown implements the snowman.ChainVM interface -func (vm *VM) Shutdown() { +func (vm *VM) Shutdown() error { if vm.timer == nil { - return + return nil } // There is a potential deadlock if the timer is about to execute a timeout. @@ -127,9 +127,8 @@ func (vm *VM) Shutdown() { vm.ctx.Lock.Unlock() vm.timer.Stop() vm.ctx.Lock.Lock() - if err := vm.baseDB.Close(); err != nil { - vm.ctx.Log.Error("Closing the database failed with %s", err) - } + + return vm.baseDB.Close() } // BuildBlock implements the snowman.ChainVM interface diff --git a/vms/spdagvm/vm.go b/vms/spdagvm/vm.go index 55394461f7b..8c838872c1c 100644 --- a/vms/spdagvm/vm.go +++ b/vms/spdagvm/vm.go @@ -129,9 +129,9 @@ func (vm *VM) Initialize( } // Shutdown implements the avalanche.DAGVM interface -func (vm *VM) Shutdown() { +func (vm *VM) Shutdown() error { if vm.timer == nil { - return + return nil } // There is a potential deadlock if the timer is about to execute a timeout. @@ -139,9 +139,8 @@ func (vm *VM) Shutdown() { vm.ctx.Lock.Unlock() vm.timer.Stop() vm.ctx.Lock.Lock() - if err := vm.baseDB.Close(); err != nil { - vm.ctx.Log.Error("Closing the database failed with %s", err) - } + + return vm.baseDB.Close() } // CreateHandlers makes new service objects with references to the vm From 232f962d4b2fb1e8d520d55eef5cc2e172aacb30 Mon Sep 17 00:00:00 2001 From: Collin Montag Date: Thu, 28 May 2020 23:50:33 -0400 Subject: [PATCH 07/29] updated tests --- vms/avm/prefixed_state.go | 2 +- vms/avm/prefixed_state_test.go | 9 +++-- vms/avm/state_test.go | 53 +++++++++++++++++++--------- vms/components/ava/prefixed_state.go | 2 +- vms/components/ava/state.go | 7 ++++ 5 files changed, 52 insertions(+), 21 deletions(-) diff --git a/vms/avm/prefixed_state.go b/vms/avm/prefixed_state.go index 862771a6d17..2b19100405c 100644 --- a/vms/avm/prefixed_state.go +++ b/vms/avm/prefixed_state.go @@ -77,7 +77,7 @@ func (s *prefixedState) SetDBInitialized(status choices.Status) error { // Funds returns the mapping from the 32 byte representation of an address to a // list of utxo IDs that reference the address. func (s *prefixedState) Funds(id ids.ID) ([]ids.ID, error) { - return s.state.IDs(uniqueID(id, fundsID, s.funds)) + return s.state.IDs(id) } // SpendUTXO consumes the provided utxo. diff --git a/vms/avm/prefixed_state_test.go b/vms/avm/prefixed_state_test.go index 4eea4048a60..6ab50c34176 100644 --- a/vms/avm/prefixed_state_test.go +++ b/vms/avm/prefixed_state_test.go @@ -182,8 +182,11 @@ func TestPrefixedFundingAddresses(t *testing.T) { if err := state.SpendUTXO(utxo.InputID()); err != nil { t.Fatal(err) } - _, err = state.Funds(ids.NewID(hashing.ComputeHash256Array([]byte{0}))) - if err == nil { - t.Fatalf("Should have returned no utxoIDs") + funds, err = state.Funds(ids.NewID(hashing.ComputeHash256Array([]byte{0}))) + if err != nil { + t.Fatal(err) + } + if len(funds) != 0 { + t.Fatalf("Should have returned 0 utxoIDs") } } diff --git a/vms/avm/state_test.go b/vms/avm/state_test.go index de5810be2dc..15c8ec747dc 100644 --- a/vms/avm/state_test.go +++ b/vms/avm/state_test.go @@ -23,9 +23,9 @@ func TestStateIDs(t *testing.T) { state := vm.state.state - id0 := ids.NewID([32]byte{0xff, 0}) - id1 := ids.NewID([32]byte{0xff, 0}) - id2 := ids.NewID([32]byte{0xff, 0}) + id0 := ids.NewID([32]byte{0x00, 0}) + id1 := ids.NewID([32]byte{0x01, 0}) + id2 := ids.NewID([32]byte{0x02, 0}) if _, err := state.IDs(ids.Empty); err != nil { t.Fatal(err) @@ -47,6 +47,7 @@ func TestStateIDs(t *testing.T) { t.Fatalf("Returned the wrong number of ids") } + ids.SortIDs(result) for i, resultID := range result { expectedID := expected[i] if !expectedID.Equals(resultID) { @@ -54,6 +55,21 @@ func TestStateIDs(t *testing.T) { } } + for _, id := range expected { + if err := state.RemoveID(ids.Empty, id); err != nil { + t.Fatal(err) + } + } + + result, err = state.IDs(ids.Empty) + if err != nil { + t.Fatal(err) + } + + if len(result) != 0 { + t.Fatalf("Should have returned 0 IDs") + } + expected = []ids.ID{id1, id2} for _, id := range expected { if err := state.AddID(ids.Empty, id); err != nil { @@ -70,6 +86,7 @@ func TestStateIDs(t *testing.T) { t.Fatalf("Returned the wrong number of ids") } + ids.SortIDs(result) for i, resultID := range result { expectedID := expected[i] if !expectedID.Equals(resultID) { @@ -88,6 +105,7 @@ func TestStateIDs(t *testing.T) { t.Fatalf("Returned the wrong number of ids") } + ids.SortIDs(result) for i, resultID := range result { expectedID := expected[i] if !expectedID.Equals(resultID) { @@ -99,32 +117,35 @@ func TestStateIDs(t *testing.T) { t.Fatal(err) } - result, err = state.IDs(ids.Empty) - if err == nil { - t.Fatalf("Should have errored during cache lookup") + statusResult, err := state.Status(ids.Empty) + if err != nil { + t.Fatal(err) + } + if statusResult != choices.Accepted { + t.Fatalf("Should have returned the %s status", choices.Accepted) } - state.Cache.Flush() - - result, err = state.IDs(ids.Empty) - if err == nil { - t.Fatalf("Should have errored during parsing") + for _, id := range expected { + if err := state.RemoveID(ids.Empty, id); err != nil { + t.Fatal(err) + } } - statusResult, err := state.Status(ids.Empty) + result, err = state.IDs(ids.Empty) if err != nil { t.Fatal(err) } - if statusResult != choices.Accepted { - t.Fatalf("Should have returned the %s status", choices.Accepted) + + if len(result) != 0 { + t.Fatalf("Should have returned 0 IDs") } if err := state.AddID(ids.Empty, ids.ID{}); err == nil { t.Fatalf("Should have errored during serialization") } - if _, err := state.IDs(ids.Empty); err == nil { - t.Fatalf("Should have errored when reading ids") + if err := state.RemoveID(ids.Empty, ids.ID{}); err == nil { + t.Fatalf("Should have errored during serialization") } } diff --git a/vms/components/ava/prefixed_state.go b/vms/components/ava/prefixed_state.go index 2fcd9aae17b..15e931c690b 100644 --- a/vms/components/ava/prefixed_state.go +++ b/vms/components/ava/prefixed_state.go @@ -47,7 +47,7 @@ func (s *chainState) UTXO(id ids.ID) (*UTXO, error) { // Funds returns the mapping from the 32 byte representation of an // address to a list of utxo IDs that reference the address. func (s *chainState) Funds(id ids.ID) ([]ids.ID, error) { - return s.IDs(UniqueID(id, s.fundsIDPrefix, s.fundsID)) + return s.IDs(id) } // SpendUTXO consumes the provided platform utxo. diff --git a/vms/components/ava/state.go b/vms/components/ava/state.go index 8c8129fdd34..997d0b43175 100644 --- a/vms/components/ava/state.go +++ b/vms/components/ava/state.go @@ -16,6 +16,7 @@ import ( var ( errCacheTypeMismatch = errors.New("type returned from cache doesn't match the expected type") + errZeroID = errors.New("database key ID value not initialized") ) // UniqueID returns a unique identifier @@ -134,12 +135,18 @@ func (s *State) IDs(id ids.ID) ([]ids.ID, error) { // AddID saves an ID to the prefixed database func (s *State) AddID(id ids.ID, key ids.ID) error { + if key.IsZero() { + return errZeroID + } db := prefixdb.New(id.Bytes(), s.DB) return db.Put(key.Bytes(), nil) } // RemoveID removes an ID from the prefixed database func (s *State) RemoveID(id ids.ID, key ids.ID) error { + if key.IsZero() { + return errZeroID + } db := prefixdb.New(id.Bytes(), s.DB) return db.Delete(key.Bytes()) } From 33eca06ca2ff0899935281cfd8625158b19eff1d Mon Sep 17 00:00:00 2001 From: Collin Montag Date: Fri, 29 May 2020 00:03:14 -0400 Subject: [PATCH 08/29] state test pruning --- vms/avm/state_test.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/vms/avm/state_test.go b/vms/avm/state_test.go index 15c8ec747dc..1feabab0dc1 100644 --- a/vms/avm/state_test.go +++ b/vms/avm/state_test.go @@ -177,13 +177,6 @@ func TestStateStatuses(t *testing.T) { if err := state.AddID(ids.Empty, ids.Empty); err != nil { t.Fatal(err) } - if _, err := state.Status(ids.Empty); err == nil { - t.Fatalf("Should have errored when reading ids") - } - - if err := state.SetStatus(ids.Empty, choices.Accepted); err != nil { - t.Fatal(err) - } status, err = state.Status(ids.Empty) if err != nil { From 312e17bfb3c62cc9bff5c80251501f753c2d0270 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Fri, 29 May 2020 12:13:05 -0400 Subject: [PATCH 09/29] Fixed deadlock issue --- snow/networking/router/handler.go | 4 ++-- snow/networking/router/subnet_router.go | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/snow/networking/router/handler.go b/snow/networking/router/handler.go index b2ca7103604..920371471e8 100644 --- a/snow/networking/router/handler.go +++ b/snow/networking/router/handler.go @@ -51,7 +51,7 @@ func (h *Handler) Dispatch() { log.Debug("dropping message due to closing:\n%s", msg) continue } - if !h.dispatchMsg(msg) { + if h.dispatchMsg(msg) { closing = true } case msg := <-h.msgChan: @@ -59,7 +59,7 @@ func (h *Handler) Dispatch() { log.Debug("dropping internal message due to closing:\n%s", msg) continue } - if !h.dispatchMsg(message{messageType: notifyMsg, notification: msg}) { + if h.dispatchMsg(message{messageType: notifyMsg, notification: msg}) { closing = true } } diff --git a/snow/networking/router/subnet_router.go b/snow/networking/router/subnet_router.go index 66f1341b0ff..bf7c2eba714 100644 --- a/snow/networking/router/subnet_router.go +++ b/snow/networking/router/subnet_router.go @@ -263,11 +263,12 @@ func (sr *ChainRouter) Shutdown() { chain.Shutdown() close(chain.msgs) } - for _, chain := range sr.chains { - chain.wg.Wait() - } sr.chains = map[[32]byte]*Handler{} + prevChains := sr.chains sr.lock.Unlock() + for _, chain := range prevChains { + chain.wg.Wait() + } sr.gossiper.Stop() } From bdf9f27f7d12d66d8e3db3eee59726a3c8a4872c Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Fri, 29 May 2020 15:03:00 -0400 Subject: [PATCH 10/29] Removed memory leak in bootstrapping --- snow/engine/avalanche/bootstrapper.go | 34 ++++++++++++++++++++------- snow/engine/snowman/bootstrapper.go | 30 ++++++++++++++++------- snow/networking/router/handler.go | 2 +- 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/snow/engine/avalanche/bootstrapper.go b/snow/engine/avalanche/bootstrapper.go index 9a8ae9e2f50..95cec3b5138 100644 --- a/snow/engine/avalanche/bootstrapper.go +++ b/snow/engine/avalanche/bootstrapper.go @@ -4,6 +4,8 @@ package avalanche import ( + "fmt" + "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/choices" "github.com/ava-labs/gecko/snow/consensus/avalanche" @@ -89,7 +91,9 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { // ForceAccepted ... func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) error { for _, vtxID := range acceptedContainerIDs.List() { - b.fetch(vtxID) + if err := b.fetch(vtxID); err != nil { + return err + } } if numPending := b.pending.Len(); numPending == 0 { @@ -135,17 +139,17 @@ func (b *bootstrapper) GetFailed(vdr ids.ShortID, requestID uint32) error { return nil } -func (b *bootstrapper) fetch(vtxID ids.ID) { +func (b *bootstrapper) fetch(vtxID ids.ID) error { if b.pending.Contains(vtxID) { - return + return nil } vtx, err := b.State.GetVertex(vtxID) if err != nil { b.sendRequest(vtxID) - return + return nil } - b.storeVertex(vtx) + return b.storeVertex(vtx) } func (b *bootstrapper) sendRequest(vtxID ids.ID) { @@ -167,7 +171,9 @@ func (b *bootstrapper) sendRequest(vtxID ids.ID) { } func (b *bootstrapper) addVertex(vtx avalanche.Vertex) error { - b.storeVertex(vtx) + if err := b.storeVertex(vtx); err != nil { + return err + } if numPending := b.pending.Len(); numPending == 0 { return b.finish() @@ -175,7 +181,7 @@ func (b *bootstrapper) addVertex(vtx avalanche.Vertex) error { return nil } -func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { +func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) error { vts := []avalanche.Vertex{vtx} b.numFetched++ if b.numFetched%2500 == 0 { // perioidcally inform user of progress @@ -204,6 +210,9 @@ func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { } else { b.BootstrapConfig.Context.Log.Verbo("couldn't push to vtxBlocked") } + if err := b.VtxBlocked.Commit(); err != nil { + return err + } for _, tx := range vtx.Txs() { if err := b.TxBlocked.Push(&txJob{ log: b.BootstrapConfig.Context.Log, @@ -216,6 +225,9 @@ func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { b.BootstrapConfig.Context.Log.Verbo("couldn't push to txBlocked") } } + if err := b.TxBlocked.Commit(); err != nil { + return err + } for _, parent := range vtx.Parents() { if parentID := parent.ID(); !b.seen.Contains(parentID) { b.seen.Add(parentID) @@ -223,14 +235,15 @@ func (b *bootstrapper) storeVertex(vtx avalanche.Vertex) { } } case choices.Accepted: - b.BootstrapConfig.Context.Log.Verbo("Bootstrapping confirmed %s", vtxID) + b.BootstrapConfig.Context.Log.Verbo("bootstrapping confirmed %s", vtxID) case choices.Rejected: - b.BootstrapConfig.Context.Log.Error("Bootstrapping wants to accept %s, however it was previously rejected", vtxID) + return fmt.Errorf("bootstrapping wants to accept %s, however it was previously rejected", vtxID) } } numPending := b.pending.Len() b.numBSPendingRequests.Set(float64(numPending)) + return nil } func (b *bootstrapper) finish() error { @@ -263,6 +276,9 @@ func (b *bootstrapper) executeAll(jobs *queue.Jobs, numBlocked prometheus.Gauge) b.BootstrapConfig.Context.Log.Error("Error executing: %s", err) return err } + if err := jobs.Commit(); err != nil { + return err + } } return nil } diff --git a/snow/engine/snowman/bootstrapper.go b/snow/engine/snowman/bootstrapper.go index 14098283563..fd52edb6ec5 100644 --- a/snow/engine/snowman/bootstrapper.go +++ b/snow/engine/snowman/bootstrapper.go @@ -4,6 +4,8 @@ package snowman import ( + "fmt" + "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow/choices" "github.com/ava-labs/gecko/snow/consensus/snowman" @@ -75,7 +77,9 @@ func (b *bootstrapper) FilterAccepted(containerIDs ids.Set) ids.Set { // ForceAccepted ... func (b *bootstrapper) ForceAccepted(acceptedContainerIDs ids.Set) error { for _, blkID := range acceptedContainerIDs.List() { - b.fetch(blkID) + if err := b.fetch(blkID); err != nil { + return err + } } if numPending := b.pending.Len(); numPending == 0 { @@ -124,17 +128,17 @@ func (b *bootstrapper) GetFailed(vdr ids.ShortID, requestID uint32) error { return nil } -func (b *bootstrapper) fetch(blkID ids.ID) { +func (b *bootstrapper) fetch(blkID ids.ID) error { if b.pending.Contains(blkID) { - return + return nil } blk, err := b.VM.GetBlock(blkID) if err != nil { b.sendRequest(blkID) - return + return nil } - b.storeBlock(blk) + return b.storeBlock(blk) } func (b *bootstrapper) sendRequest(blkID ids.ID) { @@ -156,7 +160,9 @@ func (b *bootstrapper) sendRequest(blkID ids.ID) { } func (b *bootstrapper) addBlock(blk snowman.Block) error { - b.storeBlock(blk) + if err := b.storeBlock(blk); err != nil { + return err + } if numPending := b.pending.Len(); numPending == 0 { return b.finish() @@ -164,7 +170,7 @@ func (b *bootstrapper) addBlock(blk snowman.Block) error { return nil } -func (b *bootstrapper) storeBlock(blk snowman.Block) { +func (b *bootstrapper) storeBlock(blk snowman.Block) error { status := blk.Status() blkID := blk.ID() for status == choices.Processing { @@ -178,6 +184,10 @@ func (b *bootstrapper) storeBlock(blk snowman.Block) { b.numBlocked.Inc() } + if err := b.Blocked.Commit(); err != nil { + return err + } + blk = blk.Parent() status = blk.Status() blkID = blk.ID() @@ -189,11 +199,12 @@ func (b *bootstrapper) storeBlock(blk snowman.Block) { case choices.Accepted: b.BootstrapConfig.Context.Log.Verbo("Bootstrapping confirmed %s", blkID) case choices.Rejected: - b.BootstrapConfig.Context.Log.Error("Bootstrapping wants to accept %s, however it was previously rejected", blkID) + return fmt.Errorf("bootstrapping wants to accept %s, however it was previously rejected", blkID) } numPending := b.pending.Len() b.numPendingRequests.Set(float64(numPending)) + return nil } func (b *bootstrapper) finish() error { @@ -223,6 +234,9 @@ func (b *bootstrapper) executeAll(jobs *queue.Jobs, numBlocked prometheus.Gauge) if err := jobs.Execute(job); err != nil { return err } + if err := jobs.Commit(); err != nil { + return err + } } return nil } diff --git a/snow/networking/router/handler.go b/snow/networking/router/handler.go index 920371471e8..d2de1d6395b 100644 --- a/snow/networking/router/handler.go +++ b/snow/networking/router/handler.go @@ -120,7 +120,7 @@ func (h *Handler) dispatchMsg(msg message) bool { } if err != nil { - ctx.Log.Error("fatal error occurred on chain %s, forcing a shutdown due to %s", err) + ctx.Log.Fatal("forcing chain to shutdown due to %s", err) } return done || err != nil } From 4142160b8c5854f14780ee480f6f646e50b82a66 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Fri, 29 May 2020 16:57:08 -0400 Subject: [PATCH 11/29] add address that staked funds/reward are sent to to getCurrentValidators and getPendingValidators --- vms/platformvm/service.go | 21 +++++++++++++++++++++ vms/platformvm/static_service.go | 7 ++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index 00ee665b873..3ac8f7b58b2 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -148,11 +148,22 @@ func (service *Service) GetCurrentValidators(_ *http.Request, args *GetCurrentVa vdr := tx.Vdr() weight := json.Uint64(vdr.Weight()) if args.SubnetID.Equals(DefaultSubnetID) { + var address ids.ShortID + switch tx := tx.(type) { + case *addDefaultSubnetValidatorTx: + address = tx.Destination + case *addDefaultSubnetDelegatorTx: + address = tx.Destination + default: // Shouldn't happen + service.vm.Ctx.Log.Debug("unexpected type in currentValidators list") + } + reply.Validators[i] = APIValidator{ ID: vdr.ID(), StartTime: json.Uint64(tx.StartTime().Unix()), EndTime: json.Uint64(tx.EndTime().Unix()), StakeAmount: &weight, + Address: &address, } } else { reply.Validators[i] = APIValidator{ @@ -197,11 +208,21 @@ func (service *Service) GetPendingValidators(_ *http.Request, args *GetPendingVa vdr := tx.Vdr() weight := json.Uint64(vdr.Weight()) if args.SubnetID.Equals(DefaultSubnetID) { + var address ids.ShortID + switch tx := tx.(type) { + case *addDefaultSubnetValidatorTx: + address = tx.Destination + case *addDefaultSubnetDelegatorTx: + address = tx.Destination + default: // Shouldn't happen + service.vm.Ctx.Log.Debug("unexpected type in currentValidators list") + } reply.Validators[i] = APIValidator{ ID: vdr.ID(), StartTime: json.Uint64(tx.StartTime().Unix()), EndTime: json.Uint64(tx.EndTime().Unix()), StakeAmount: &weight, + Address: &address, } } else { reply.Validators[i] = APIValidator{ diff --git a/vms/platformvm/static_service.go b/vms/platformvm/static_service.go index 86af00538ed..6bcf6f972d1 100644 --- a/vms/platformvm/static_service.go +++ b/vms/platformvm/static_service.go @@ -40,13 +40,14 @@ type APIAccount struct { // [Amount] is the amount of $AVA being staked. // [Endtime] is the Unix time repr. of when they are done staking // [ID] is the node ID of the staker -// [Destination] is the address where the staked $AVA (and, if applicable, reward) +// [Address] is the address where the staked AVA (and, if applicable, reward) // is sent when this staker is done staking. type APIValidator struct { StartTime json.Uint64 `json:"startTime"` EndTime json.Uint64 `json:"endTime"` Weight *json.Uint64 `json:"weight,omitempty"` StakeAmount *json.Uint64 `json:"stakeAmount,omitempty"` + Address *ids.ShortID `json:"address,omitempty"` ID ids.ShortID `json:"id"` } @@ -137,8 +138,8 @@ func (*StaticService) BuildGenesis(_ *http.Request, args *BuildGenesisArgs, repl return errAccountHasNoValue } accounts = append(accounts, newAccount( - account.Address, // ID - 0, // nonce + account.Address, // ID + 0, // nonce uint64(account.Balance), // balance )) } From 18e1a5ca4f9d4f0c7240f19dc1b0bd466794574f Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Fri, 29 May 2020 18:08:44 -0400 Subject: [PATCH 12/29] await chain shutdowns correctly --- node/node.go | 1 + snow/networking/router/handler.go | 5 ++++- snow/networking/router/subnet_router.go | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/node/node.go b/node/node.go index dfa9549de1f..a2d9c2e9fad 100644 --- a/node/node.go +++ b/node/node.go @@ -575,4 +575,5 @@ func (n *Node) Shutdown() { n.Net.Close() n.chainManager.Shutdown() utils.ClearSignals(n.nodeCloser) + n.Log.Info("node shut down successfully") } diff --git a/snow/networking/router/handler.go b/snow/networking/router/handler.go index d2de1d6395b..b7b93f43438 100644 --- a/snow/networking/router/handler.go +++ b/snow/networking/router/handler.go @@ -38,7 +38,10 @@ func (h *Handler) Context() *snow.Context { return h.engine.Context() } // and, when they arrive, sends them to the consensus engine func (h *Handler) Dispatch() { log := h.Context().Log - defer h.wg.Done() + defer func() { + log.Info("finished shutting down chain") + h.wg.Done() + }() closing := false for { diff --git a/snow/networking/router/subnet_router.go b/snow/networking/router/subnet_router.go index bf7c2eba714..075a00bcd8b 100644 --- a/snow/networking/router/subnet_router.go +++ b/snow/networking/router/subnet_router.go @@ -263,8 +263,8 @@ func (sr *ChainRouter) Shutdown() { chain.Shutdown() close(chain.msgs) } - sr.chains = map[[32]byte]*Handler{} prevChains := sr.chains + sr.chains = map[[32]byte]*Handler{} sr.lock.Unlock() for _, chain := range prevChains { chain.wg.Wait() From dedc3b20a6e05bd5e1a0e507eb7cf84dbaac12dc Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Fri, 29 May 2020 18:32:17 -0400 Subject: [PATCH 13/29] Added shutdown timeouts --- chains/manager.go | 3 +- snow/engine/avalanche/bootstrapper_test.go | 2 +- snow/engine/snowman/bootstrapper_test.go | 2 +- snow/networking/router/handler.go | 9 ++--- snow/networking/router/router.go | 7 +++- snow/networking/router/subnet_router.go | 42 +++++++++++++++++----- snow/networking/sender/sender_test.go | 2 +- vms/platformvm/vm_test.go | 2 +- vms/spchainvm/consensus_benchmark_test.go | 4 +-- 9 files changed, 51 insertions(+), 22 deletions(-) diff --git a/chains/manager.go b/chains/manager.go index 552bdea9891..044cba548d0 100644 --- a/chains/manager.go +++ b/chains/manager.go @@ -40,6 +40,7 @@ const ( defaultChannelSize = 1000 requestTimeout = 2 * time.Second gossipFrequency = 10 * time.Second + shutdownTimeout = 1 * time.Second ) // Manager manages the chains running on this node. @@ -144,7 +145,7 @@ func New( timeoutManager.Initialize(requestTimeout) go log.RecoverAndPanic(timeoutManager.Dispatch) - router.Initialize(log, &timeoutManager, gossipFrequency) + router.Initialize(log, &timeoutManager, gossipFrequency, shutdownTimeout) m := &manager{ stakingEnabled: stakingEnabled, diff --git a/snow/engine/avalanche/bootstrapper_test.go b/snow/engine/avalanche/bootstrapper_test.go index f5cc3b50bf5..0298a7891ae 100644 --- a/snow/engine/avalanche/bootstrapper_test.go +++ b/snow/engine/avalanche/bootstrapper_test.go @@ -60,7 +60,7 @@ func newConfig(t *testing.T) (BootstrapConfig, ids.ShortID, *common.SenderTest, handler.Initialize(engine, make(chan common.Message), 1) timeouts.Initialize(0) - router.Initialize(ctx.Log, timeouts, time.Hour) + router.Initialize(ctx.Log, timeouts, time.Hour, time.Second) vtxBlocker, _ := queue.New(prefixdb.New([]byte("vtx"), db)) txBlocker, _ := queue.New(prefixdb.New([]byte("tx"), db)) diff --git a/snow/engine/snowman/bootstrapper_test.go b/snow/engine/snowman/bootstrapper_test.go index 8b7d3aa8f4c..ce24bc5d529 100644 --- a/snow/engine/snowman/bootstrapper_test.go +++ b/snow/engine/snowman/bootstrapper_test.go @@ -54,7 +54,7 @@ func newConfig(t *testing.T) (BootstrapConfig, ids.ShortID, *common.SenderTest, handler.Initialize(engine, make(chan common.Message), 1) timeouts.Initialize(0) - router.Initialize(ctx.Log, timeouts, time.Hour) + router.Initialize(ctx.Log, timeouts, time.Hour, time.Second) blocker, _ := queue.New(db) diff --git a/snow/networking/router/handler.go b/snow/networking/router/handler.go index b7b93f43438..b6bb1cfb3bb 100644 --- a/snow/networking/router/handler.go +++ b/snow/networking/router/handler.go @@ -4,8 +4,6 @@ package router import ( - "sync" - "github.com/ava-labs/gecko/ids" "github.com/ava-labs/gecko/snow" "github.com/ava-labs/gecko/snow/engine/common" @@ -15,7 +13,7 @@ import ( // (Actually, it receives the incoming messages from a ChainRouter, but same difference) type Handler struct { msgs chan message - wg sync.WaitGroup + closed chan struct{} engine common.Engine msgChan <-chan common.Message @@ -25,10 +23,9 @@ type Handler struct { // Initialize this consensus handler func (h *Handler) Initialize(engine common.Engine, msgChan <-chan common.Message, bufferSize int) { h.msgs = make(chan message, bufferSize) + h.closed = make(chan struct{}) h.engine = engine h.msgChan = msgChan - - h.wg.Add(1) } // Context of this Handler @@ -40,7 +37,7 @@ func (h *Handler) Dispatch() { log := h.Context().Log defer func() { log.Info("finished shutting down chain") - h.wg.Done() + close(h.closed) }() closing := false diff --git a/snow/networking/router/router.go b/snow/networking/router/router.go index 93c7bcc2ff1..d0b6a54673b 100644 --- a/snow/networking/router/router.go +++ b/snow/networking/router/router.go @@ -20,7 +20,12 @@ type Router interface { AddChain(chain *Handler) RemoveChain(chainID ids.ID) Shutdown() - Initialize(log logging.Logger, timeouts *timeout.Manager, gossipFrequency time.Duration) + Initialize( + log logging.Logger, + timeouts *timeout.Manager, + gossipFrequency, + shutdownTimeout time.Duration, + ) } // ExternalRouter routes messages from the network to the diff --git a/snow/networking/router/subnet_router.go b/snow/networking/router/subnet_router.go index 075a00bcd8b..36187dc2387 100644 --- a/snow/networking/router/subnet_router.go +++ b/snow/networking/router/subnet_router.go @@ -18,11 +18,12 @@ import ( // Note that consensus engines are uniquely identified by the ID of the chain // that they are working on. type ChainRouter struct { - log logging.Logger - lock sync.RWMutex - chains map[[32]byte]*Handler - timeouts *timeout.Manager - gossiper *timer.Repeater + log logging.Logger + lock sync.RWMutex + chains map[[32]byte]*Handler + timeouts *timeout.Manager + gossiper *timer.Repeater + closeTimeout time.Duration } // Initialize the router. @@ -33,11 +34,17 @@ type ChainRouter struct { // // This router also fires a gossip event every [gossipFrequency] to the engine, // notifying the engine it should gossip it's accepted set. -func (sr *ChainRouter) Initialize(log logging.Logger, timeouts *timeout.Manager, gossipFrequency time.Duration) { +func (sr *ChainRouter) Initialize( + log logging.Logger, + timeouts *timeout.Manager, + gossipFrequency time.Duration, + closeTimeout time.Duration, +) { sr.log = log sr.chains = make(map[[32]byte]*Handler) sr.timeouts = timeouts sr.gossiper = timer.NewRepeater(sr.Gossip, gossipFrequency) + sr.closeTimeout = closeTimeout go log.RecoverAndPanic(sr.gossiper.Dispatch) } @@ -63,7 +70,15 @@ func (sr *ChainRouter) RemoveChain(chainID ids.ID) { if chain, exists := sr.chains[chainID.Key()]; exists { chain.Shutdown() close(chain.msgs) - chain.wg.Wait() + + ticker := time.NewTicker(sr.closeTimeout) + select { + case _, _ = <-chain.closed: + case <-ticker.C: + chain.Context().Log.Warn("timed out while shutting down") + } + ticker.Stop() + delete(sr.chains, chainID.Key()) } else { sr.log.Debug("message referenced a chain, %s, this node doesn't validate", chainID) @@ -266,9 +281,20 @@ func (sr *ChainRouter) Shutdown() { prevChains := sr.chains sr.chains = map[[32]byte]*Handler{} sr.lock.Unlock() + + ticker := time.NewTicker(sr.closeTimeout) + timedout := false for _, chain := range prevChains { - chain.wg.Wait() + select { + case _, _ = <-chain.closed: + case <-ticker.C: + timedout = true + } + } + if timedout { + sr.log.Warn("timed out while shutting down the chains") } + ticker.Stop() sr.gossiper.Stop() } diff --git a/snow/networking/sender/sender_test.go b/snow/networking/sender/sender_test.go index a8361d5f7fc..45286fa1234 100644 --- a/snow/networking/sender/sender_test.go +++ b/snow/networking/sender/sender_test.go @@ -37,7 +37,7 @@ func TestTimeout(t *testing.T) { go tm.Dispatch() chainRouter := router.ChainRouter{} - chainRouter.Initialize(logging.NoLog{}, &tm, time.Hour) + chainRouter.Initialize(logging.NoLog{}, &tm, time.Hour, time.Second) sender := Sender{} sender.Initialize(snow.DefaultContextTest(), &ExternalSenderTest{}, &chainRouter, &tm) diff --git a/vms/platformvm/vm_test.go b/vms/platformvm/vm_test.go index 10b08a07875..fd782f718eb 100644 --- a/vms/platformvm/vm_test.go +++ b/vms/platformvm/vm_test.go @@ -1560,7 +1560,7 @@ func TestBootstrapPartiallyAccepted(t *testing.T) { go timeoutManager.Dispatch() chainRouter := &router.ChainRouter{} - chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) + chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour, time.Second) externalSender := &sender.ExternalSenderTest{T: t} externalSender.Default(true) diff --git a/vms/spchainvm/consensus_benchmark_test.go b/vms/spchainvm/consensus_benchmark_test.go index b304320e02a..5dcc247f60c 100644 --- a/vms/spchainvm/consensus_benchmark_test.go +++ b/vms/spchainvm/consensus_benchmark_test.go @@ -61,7 +61,7 @@ func ConsensusLeader(numBlocks, numTxsPerBlock int, b *testing.B) { go timeoutManager.Dispatch() chainRouter := &router.ChainRouter{} - chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) + chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour, time.Second) // Initialize the VM vm := &VM{} @@ -189,7 +189,7 @@ func ConsensusFollower(numBlocks, numTxsPerBlock int, b *testing.B) { go timeoutManager.Dispatch() chainRouter := &router.ChainRouter{} - chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour) + chainRouter.Initialize(logging.NoLog{}, &timeoutManager, time.Hour, time.Second) wg := sync.WaitGroup{} wg.Add(numBlocks) From 97754e2545fbfe4228c73c801fe4d8ff4468e040 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 12:44:41 -0400 Subject: [PATCH 14/29] Added tests for separation between the avm and the platformvm --- vms/avm/prefixed_state.go | 9 +-- vms/avm/prefixed_state_test.go | 4 +- vms/avm/verifiable_test.go | 14 ----- vms/avm/vm.go | 1 - vms/components/ava/prefixed_state.go | 4 +- vms/components/ava/prefixed_state_test.go | 72 +++++++++++++++++++++++ vms/components/ava/test_verifiable.go | 10 ++++ 7 files changed, 90 insertions(+), 24 deletions(-) delete mode 100644 vms/avm/verifiable_test.go create mode 100644 vms/components/ava/prefixed_state_test.go diff --git a/vms/avm/prefixed_state.go b/vms/avm/prefixed_state.go index 2b19100405c..5f29791167f 100644 --- a/vms/avm/prefixed_state.go +++ b/vms/avm/prefixed_state.go @@ -15,7 +15,6 @@ const ( txID uint64 = iota utxoID txStatusID - fundsID dbInitializedID ) @@ -28,8 +27,8 @@ var ( type prefixedState struct { state *state - tx, utxo, txStatus, funds cache.Cacher - uniqueTx cache.Deduplicator + tx, utxo, txStatus cache.Cacher + uniqueTx cache.Deduplicator } // UniqueTx de-duplicates the transaction. @@ -76,9 +75,7 @@ func (s *prefixedState) SetDBInitialized(status choices.Status) error { // Funds returns the mapping from the 32 byte representation of an address to a // list of utxo IDs that reference the address. -func (s *prefixedState) Funds(id ids.ID) ([]ids.ID, error) { - return s.state.IDs(id) -} +func (s *prefixedState) Funds(id ids.ID) ([]ids.ID, error) { return s.state.IDs(id) } // SpendUTXO consumes the provided utxo. func (s *prefixedState) SpendUTXO(utxoID ids.ID) error { diff --git a/vms/avm/prefixed_state_test.go b/vms/avm/prefixed_state_test.go index 6ab50c34176..74c4b53917d 100644 --- a/vms/avm/prefixed_state_test.go +++ b/vms/avm/prefixed_state_test.go @@ -151,7 +151,7 @@ func TestPrefixedFundingAddresses(t *testing.T) { state := vm.state - vm.codec.RegisterType(&testAddressable{}) + vm.codec.RegisterType(&ava.TestAddressable{}) utxo := &ava.UTXO{ UTXOID: ava.UTXOID{ @@ -159,7 +159,7 @@ func TestPrefixedFundingAddresses(t *testing.T) { OutputIndex: 1, }, Asset: ava.Asset{ID: ids.Empty}, - Out: &testAddressable{ + Out: &ava.TestAddressable{ Addrs: [][]byte{ []byte{0}, }, diff --git a/vms/avm/verifiable_test.go b/vms/avm/verifiable_test.go deleted file mode 100644 index 6e6337af5f0..00000000000 --- a/vms/avm/verifiable_test.go +++ /dev/null @@ -1,14 +0,0 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package avm - -import "github.com/ava-labs/gecko/vms/components/ava" - -type testAddressable struct { - ava.TestTransferable `serialize:"true"` - - Addrs [][]byte `serialize:"true"` -} - -func (a *testAddressable) Addresses() [][]byte { return a.Addrs } diff --git a/vms/avm/vm.go b/vms/avm/vm.go index 180f4ae8185..de37b5710b8 100644 --- a/vms/avm/vm.go +++ b/vms/avm/vm.go @@ -171,7 +171,6 @@ func (vm *VM) Initialize( tx: &cache.LRU{Size: idCacheSize}, utxo: &cache.LRU{Size: idCacheSize}, txStatus: &cache.LRU{Size: idCacheSize}, - funds: &cache.LRU{Size: idCacheSize}, uniqueTx: &cache.EvictableLRU{Size: txCacheSize}, } diff --git a/vms/components/ava/prefixed_state.go b/vms/components/ava/prefixed_state.go index 15e931c690b..92b3491379d 100644 --- a/vms/components/ava/prefixed_state.go +++ b/vms/components/ava/prefixed_state.go @@ -47,7 +47,7 @@ func (s *chainState) UTXO(id ids.ID) (*UTXO, error) { // Funds returns the mapping from the 32 byte representation of an // address to a list of utxo IDs that reference the address. func (s *chainState) Funds(id ids.ID) ([]ids.ID, error) { - return s.IDs(id) + return s.IDs(UniqueID(id, s.fundsIDPrefix, s.fundsID)) } // SpendUTXO consumes the provided platform utxo. @@ -97,6 +97,7 @@ func (s *chainState) setStatus(id ids.ID, status choices.Status) error { func (s *chainState) removeUTXO(addrs [][]byte, utxoID ids.ID) error { for _, addr := range addrs { addrID := ids.NewID(hashing.ComputeHash256Array(addr)) + addrID = UniqueID(addrID, s.fundsIDPrefix, s.fundsID) if err := s.RemoveID(addrID, utxoID); err != nil { return err } @@ -107,6 +108,7 @@ func (s *chainState) removeUTXO(addrs [][]byte, utxoID ids.ID) error { func (s *chainState) addUTXO(addrs [][]byte, utxoID ids.ID) error { for _, addr := range addrs { addrID := ids.NewID(hashing.ComputeHash256Array(addr)) + addrID = UniqueID(addrID, s.fundsIDPrefix, s.fundsID) if err := s.AddID(addrID, utxoID); err != nil { return err } diff --git a/vms/components/ava/prefixed_state_test.go b/vms/components/ava/prefixed_state_test.go new file mode 100644 index 00000000000..06cb1df1228 --- /dev/null +++ b/vms/components/ava/prefixed_state_test.go @@ -0,0 +1,72 @@ +// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package ava + +import ( + "testing" + + "github.com/ava-labs/gecko/database/memdb" + "github.com/ava-labs/gecko/ids" + "github.com/ava-labs/gecko/utils/hashing" + "github.com/ava-labs/gecko/vms/components/codec" + "github.com/stretchr/testify/assert" +) + +func TestPrefixedFunds(t *testing.T) { + db := memdb.New() + cc := codec.NewDefault() + + cc.RegisterType(&TestAddressable{}) + + st := NewPrefixedState(db, cc) + + avmUTXO := &UTXO{ + UTXOID: UTXOID{ + TxID: ids.Empty, + OutputIndex: 0, + }, + Asset: Asset{ + ID: ids.Empty, + }, + Out: &TestAddressable{ + Addrs: [][]byte{ + []byte{0}, + }, + }, + } + + platformUTXO := &UTXO{ + UTXOID: UTXOID{ + TxID: ids.Empty, + OutputIndex: 1, + }, + Asset: Asset{ + ID: ids.Empty, + }, + Out: &TestAddressable{ + Addrs: [][]byte{ + []byte{0}, + }, + }, + } + + assert.NoError(t, st.FundAVMUTXO(avmUTXO)) + assert.NoError(t, st.FundPlatformUTXO(platformUTXO)) + + addrID := ids.NewID(hashing.ComputeHash256Array([]byte{0})) + + avmUTXOIDs, err := st.AVMFunds(addrID) + assert.NoError(t, err) + assert.Equal(t, []ids.ID{avmUTXO.InputID()}, avmUTXOIDs) + + platformUTXOIDs, err := st.PlatformFunds(addrID) + assert.NoError(t, err) + assert.Equal(t, []ids.ID{platformUTXO.InputID()}, platformUTXOIDs) + + assert.NoError(t, st.SpendAVMUTXO(avmUTXO.InputID())) + + avmUTXOIDs, err = st.AVMFunds(addrID) + assert.NoError(t, err) + assert.Len(t, avmUTXOIDs, 0) +} diff --git a/vms/components/ava/test_verifiable.go b/vms/components/ava/test_verifiable.go index 34dce1d99fd..7282c9488bb 100644 --- a/vms/components/ava/test_verifiable.go +++ b/vms/components/ava/test_verifiable.go @@ -18,3 +18,13 @@ type TestTransferable struct { // Amount ... func (t *TestTransferable) Amount() uint64 { return t.Val } + +// TestAddressable ... +type TestAddressable struct { + TestTransferable `serialize:"true"` + + Addrs [][]byte `serialize:"true"` +} + +// Addresses ... +func (a *TestAddressable) Addresses() [][]byte { return a.Addrs } From d7e67f00b70f3db6dbe447d2aa70c9721e79fd8f Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 13:35:46 -0400 Subject: [PATCH 15/29] Bumped version number --- main/params.go | 2 +- node/node.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/main/params.go b/main/params.go index 70506521a3d..e0488b72ca4 100644 --- a/main/params.go +++ b/main/params.go @@ -29,7 +29,7 @@ import ( ) const ( - dbVersion = "v0.3.0" + dbVersion = "v0.4.0" ) // Results of parsing the CLI diff --git a/node/node.go b/node/node.go index a2d9c2e9fad..837d0169cd5 100644 --- a/node/node.go +++ b/node/node.go @@ -55,7 +55,7 @@ const ( var ( genesisHashKey = []byte("genesisID") - nodeVersion = version.NewDefaultVersion("avalanche", 0, 3, 0) + nodeVersion = version.NewDefaultVersion("avalanche", 0, 4, 0) versionParser = version.NewDefaultParser() ) From 72f203fa0e98cfef8f5a08703d213d1dde223dad Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 14:23:08 -0400 Subject: [PATCH 16/29] Added more information to the admin.peers rpc --- api/admin/networking.go | 27 --------------------------- api/admin/service.go | 19 ++++++++----------- network/network.go | 21 ++++++++++++++------- network/peer.go | 11 +++++++++++ network/peer_id.go | 20 ++++++++++++++++++++ 5 files changed, 53 insertions(+), 45 deletions(-) delete mode 100644 api/admin/networking.go create mode 100644 network/peer_id.go diff --git a/api/admin/networking.go b/api/admin/networking.go deleted file mode 100644 index 3207e4607bc..00000000000 --- a/api/admin/networking.go +++ /dev/null @@ -1,27 +0,0 @@ -// (c) 2019-2020, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package admin - -import ( - "sort" - - "github.com/ava-labs/gecko/utils" -) - -// Peerable can return a group of peers -type Peerable interface{ IPs() []utils.IPDesc } - -// Networking provides helper methods for tracking the current network state -type Networking struct{ peers Peerable } - -// Peers returns the current peers -func (n *Networking) Peers() ([]string, error) { - ipDescs := n.peers.IPs() - ips := make([]string, len(ipDescs)) - for i, ipDesc := range ipDescs { - ips[i] = ipDesc.String() - } - sort.Strings(ips) - return ips, nil -} diff --git a/api/admin/service.go b/api/admin/service.go index abfbe1b1306..ba8cad2159c 100644 --- a/api/admin/service.go +++ b/api/admin/service.go @@ -11,6 +11,7 @@ import ( "github.com/ava-labs/gecko/api" "github.com/ava-labs/gecko/chains" "github.com/ava-labs/gecko/ids" + "github.com/ava-labs/gecko/network" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/utils/logging" @@ -22,14 +23,14 @@ type Admin struct { nodeID ids.ShortID networkID uint32 log logging.Logger - networking Networking + networking network.Network performance Performance chainManager chains.Manager httpServer *api.Server } // NewService returns a new admin API service -func NewService(nodeID ids.ShortID, networkID uint32, log logging.Logger, chainManager chains.Manager, peers Peerable, httpServer *api.Server) *common.HTTPHandler { +func NewService(nodeID ids.ShortID, networkID uint32, log logging.Logger, chainManager chains.Manager, peers network.Network, httpServer *api.Server) *common.HTTPHandler { newServer := rpc.NewServer() codec := cjson.NewCodec() newServer.RegisterCodec(codec, "application/json") @@ -39,10 +40,8 @@ func NewService(nodeID ids.ShortID, networkID uint32, log logging.Logger, chainM networkID: networkID, log: log, chainManager: chainManager, - networking: Networking{ - peers: peers, - }, - httpServer: httpServer, + networking: peers, + httpServer: httpServer, }, "admin") return &common.HTTPHandler{Handler: newServer} } @@ -103,16 +102,14 @@ type PeersArgs struct{} // PeersReply are the results from calling Peers type PeersReply struct { - Peers []string `json:"peers"` + Peers []network.PeerID `json:"peers"` } // Peers returns the list of current validators func (service *Admin) Peers(r *http.Request, args *PeersArgs, reply *PeersReply) error { service.log.Debug("Admin: Peers called") - - peers, err := service.networking.Peers() - reply.Peers = peers - return err + reply.Peers = service.networking.Peers() + return nil } // StartCPUProfilerArgs are the arguments for calling StartCPUProfiler diff --git a/network/network.go b/network/network.go index 8a61364c36b..471de3ac53c 100644 --- a/network/network.go +++ b/network/network.go @@ -70,9 +70,9 @@ type Network interface { // The handler will initially be called with this local node's ID. RegisterHandler(h Handler) - // Returns the IPs of nodes this network is currently connected to - // externally. Thread safety must be managed internally to the network. - IPs() []utils.IPDesc + // Returns the description of the nodes this network is currently connected + // to externally. Thread safety must be managed internally to the network. + Peers() []PeerID // Close this network and all existing connections it has. Thread safety // must be managed internally to the network. Calling close multiple times @@ -511,17 +511,24 @@ func (n *network) RegisterHandler(h Handler) { } // IPs implements the Network interface -func (n *network) IPs() []utils.IPDesc { +func (n *network) Peers() []PeerID { n.stateLock.Lock() defer n.stateLock.Unlock() - ips := []utils.IPDesc(nil) + peers := []PeerID{} for _, peer := range n.peers { if peer.connected { - ips = append(ips, peer.ip) + peers = append(peers, PeerID{ + IP: peer.conn.RemoteAddr().String(), + PublicIP: peer.ip.String(), + ID: peer.id, + Version: peer.versionStr, + LastSent: time.Unix(atomic.LoadInt64(&peer.lastSent), 0), + LastReceived: time.Unix(atomic.LoadInt64(&peer.lastReceived), 0), + }) } } - return ips + return peers } // Close implements the Network interface diff --git a/network/peer.go b/network/peer.go index dfbde6379a2..5bb7601f470 100644 --- a/network/peer.go +++ b/network/peer.go @@ -8,6 +8,7 @@ import ( "math" "net" "sync" + "sync/atomic" "time" "github.com/ava-labs/gecko/ids" @@ -43,6 +44,12 @@ type peer struct { // the connection object that is used to read/write messages from conn net.Conn + + // version that the peer reported during the handshake + versionStr string + + // unix time of the last message sent and received respectively + lastSent, lastReceived int64 } // assume the stateLock is held @@ -159,6 +166,7 @@ func (p *peer) WriteMessages() { } msg = msg[written:] } + atomic.StoreInt64(&p.lastSent, p.net.clock.Time().Unix()) } } @@ -188,6 +196,7 @@ func (p *peer) send(msg Msg) bool { // assumes the stateLock is not held func (p *peer) handle(msg Msg) { p.net.heartbeat() + atomic.StoreInt64(&p.lastReceived, p.net.clock.Time().Unix()) op := msg.Op() msgMetrics := p.net.message(op) @@ -415,6 +424,8 @@ func (p *peer) version(msg Msg) { return } + p.versionStr = peerVersion.String() + p.connected = true p.net.connected(p) } diff --git a/network/peer_id.go b/network/peer_id.go new file mode 100644 index 00000000000..333a9231d4b --- /dev/null +++ b/network/peer_id.go @@ -0,0 +1,20 @@ +// (c) 2019-2020, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package network + +import ( + "time" + + "github.com/ava-labs/gecko/ids" +) + +// PeerID ... +type PeerID struct { + IP string `json:"ip"` + PublicIP string `json:"publicIP"` + ID ids.ShortID `json:"id"` + Version string `json:"version"` + LastSent time.Time `json:"lastSent"` + LastReceived time.Time `json:"lastReceived"` +} From 0c42ca883ca9933506d60c2739f778b12c27996a Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 14:36:16 -0400 Subject: [PATCH 17/29] bump coreth version --- go.mod | 2 +- scripts/build.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 1c91ead842a..d9caa7b4781 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/AppsFlyer/go-sundheit v0.2.0 github.com/allegro/bigcache v1.2.1 // indirect github.com/aristanetworks/goarista v0.0.0-20200520141224-0f14e646773f // indirect - github.com/ava-labs/coreth v0.1.0 // Added manually; don't delete + github.com/ava-labs/coreth v0.2.0 // Added manually; don't delete github.com/ava-labs/go-ethereum v1.9.3 // indirect github.com/deckarep/golang-set v1.7.1 // indirect github.com/decred/dcrd/dcrec/secp256k1 v1.0.3 diff --git a/scripts/build.sh b/scripts/build.sh index ae2b105fa11..e3d3a87eb5e 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -15,7 +15,7 @@ GECKO_PATH=$( cd "$( dirname "${BASH_SOURCE[0]}" )"; cd .. && pwd ) # Directory BUILD_DIR=$GECKO_PATH/build # Where binaries go PLUGIN_DIR="$BUILD_DIR/plugins" # Where plugin binaries (namely coreth) go -CORETH_VER="0.1.0" # Should match coreth version in go.mod +CORETH_VER="0.2.0" # Should match coreth version in go.mod CORETH_PATH="$GOPATH/pkg/mod/github.com/ava-labs/coreth@v$CORETH_VER" # Build Gecko From cea5e2d7bc0f0878ffff070133d0804276ceb8e8 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 15:50:37 -0400 Subject: [PATCH 18/29] Set up denali genesis --- genesis/config.go | 118 ++++++++++++++++++++++++++++++++++++++++ genesis/genesis_test.go | 39 +++++++++---- genesis/network_id.go | 18 ++++-- main/params.go | 2 +- 4 files changed, 161 insertions(+), 16 deletions(-) diff --git a/genesis/config.go b/genesis/config.go index ceab4d52d7c..d9a8672b68e 100644 --- a/genesis/config.go +++ b/genesis/config.go @@ -50,6 +50,122 @@ func (c *Config) init() error { // Hard coded genesis constants var ( + DenaliConfig = Config{ + MintAddresses: []string{ + "95YUFjhDG892VePMzpwKF9JzewGKvGRi3", + }, + FundedAddresses: []string{ + "9uKvvA7E35QCwLvAaohXTCfFejbf3Rv17", + "JLrYNMYXANGj43BfWXBxMMAEenUBp1Sbn", + "7TUTzwrU6nbZtWHjTHEpdneUvjKBxb3EM", + "77mPUXBdQKwQpPoX6rckCZGLGGdkuG1G6", + "4gGWdFZ4Gax1B466YKXyKRRpWLb42Afdt", + "CKTkzAPsRxCreyiDTnjGxLmjMarxF28fi", + "4ABm9gFHVtsNdcKSd1xsacFkGneSgzpaa", + "DpL8PTsrjtLzv5J8LL3D2A6YcnCTqrNH9", + "ZdhZv6oZrmXLyFDy6ovXAu6VxmbTsT2h", + "6cesTteH62Y5mLoDBUASaBvCXuL2AthL", + }, + StakerIDs: []string{ + "NX4zVkuiRJZYe6Nzzav7GXN3TakUet3Co", + "CMsa8cMw4eib1Hb8GG4xiUKAq5eE1BwUX", + "DsMP6jLhi1MkDVc3qx9xx9AAZWx8e87Jd", + "N86eodVZja3GEyZJTo3DFUPGpxEEvjGHs", + "EkKeGSLUbHrrtuayBtbwgWDRUiAziC3ao", + }, + EVMBytes: []byte{ + 0x7b, 0x22, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, + 0x22, 0x3a, 0x7b, 0x22, 0x63, 0x68, 0x61, 0x69, + 0x6e, 0x49, 0x64, 0x22, 0x3a, 0x34, 0x33, 0x31, + 0x31, 0x30, 0x2c, 0x22, 0x68, 0x6f, 0x6d, 0x65, + 0x73, 0x74, 0x65, 0x61, 0x64, 0x42, 0x6c, 0x6f, + 0x63, 0x6b, 0x22, 0x3a, 0x30, 0x2c, 0x22, 0x64, + 0x61, 0x6f, 0x46, 0x6f, 0x72, 0x6b, 0x42, 0x6c, + 0x6f, 0x63, 0x6b, 0x22, 0x3a, 0x30, 0x2c, 0x22, + 0x64, 0x61, 0x6f, 0x46, 0x6f, 0x72, 0x6b, 0x53, + 0x75, 0x70, 0x70, 0x6f, 0x72, 0x74, 0x22, 0x3a, + 0x74, 0x72, 0x75, 0x65, 0x2c, 0x22, 0x65, 0x69, + 0x70, 0x31, 0x35, 0x30, 0x42, 0x6c, 0x6f, 0x63, + 0x6b, 0x22, 0x3a, 0x30, 0x2c, 0x22, 0x65, 0x69, + 0x70, 0x31, 0x35, 0x30, 0x48, 0x61, 0x73, 0x68, + 0x22, 0x3a, 0x22, 0x30, 0x78, 0x32, 0x30, 0x38, + 0x36, 0x37, 0x39, 0x39, 0x61, 0x65, 0x65, 0x62, + 0x65, 0x61, 0x65, 0x31, 0x33, 0x35, 0x63, 0x32, + 0x34, 0x36, 0x63, 0x36, 0x35, 0x30, 0x32, 0x31, + 0x63, 0x38, 0x32, 0x62, 0x34, 0x65, 0x31, 0x35, + 0x61, 0x32, 0x63, 0x34, 0x35, 0x31, 0x33, 0x34, + 0x30, 0x39, 0x39, 0x33, 0x61, 0x61, 0x63, 0x66, + 0x64, 0x32, 0x37, 0x35, 0x31, 0x38, 0x38, 0x36, + 0x35, 0x31, 0x34, 0x66, 0x30, 0x22, 0x2c, 0x22, + 0x65, 0x69, 0x70, 0x31, 0x35, 0x35, 0x42, 0x6c, + 0x6f, 0x63, 0x6b, 0x22, 0x3a, 0x30, 0x2c, 0x22, + 0x65, 0x69, 0x70, 0x31, 0x35, 0x38, 0x42, 0x6c, + 0x6f, 0x63, 0x6b, 0x22, 0x3a, 0x30, 0x2c, 0x22, + 0x62, 0x79, 0x7a, 0x61, 0x6e, 0x74, 0x69, 0x75, + 0x6d, 0x42, 0x6c, 0x6f, 0x63, 0x6b, 0x22, 0x3a, + 0x30, 0x2c, 0x22, 0x63, 0x6f, 0x6e, 0x73, 0x74, + 0x61, 0x6e, 0x74, 0x69, 0x6e, 0x6f, 0x70, 0x6c, + 0x65, 0x42, 0x6c, 0x6f, 0x63, 0x6b, 0x22, 0x3a, + 0x30, 0x2c, 0x22, 0x70, 0x65, 0x74, 0x65, 0x72, + 0x73, 0x62, 0x75, 0x72, 0x67, 0x42, 0x6c, 0x6f, + 0x63, 0x6b, 0x22, 0x3a, 0x30, 0x7d, 0x2c, 0x22, + 0x6e, 0x6f, 0x6e, 0x63, 0x65, 0x22, 0x3a, 0x22, + 0x30, 0x78, 0x30, 0x22, 0x2c, 0x22, 0x74, 0x69, + 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x22, + 0x3a, 0x22, 0x30, 0x78, 0x30, 0x22, 0x2c, 0x22, + 0x65, 0x78, 0x74, 0x72, 0x61, 0x44, 0x61, 0x74, + 0x61, 0x22, 0x3a, 0x22, 0x30, 0x78, 0x30, 0x30, + 0x22, 0x2c, 0x22, 0x67, 0x61, 0x73, 0x4c, 0x69, + 0x6d, 0x69, 0x74, 0x22, 0x3a, 0x22, 0x30, 0x78, + 0x35, 0x66, 0x35, 0x65, 0x31, 0x30, 0x30, 0x22, + 0x2c, 0x22, 0x64, 0x69, 0x66, 0x66, 0x69, 0x63, + 0x75, 0x6c, 0x74, 0x79, 0x22, 0x3a, 0x22, 0x30, + 0x78, 0x30, 0x22, 0x2c, 0x22, 0x6d, 0x69, 0x78, + 0x48, 0x61, 0x73, 0x68, 0x22, 0x3a, 0x22, 0x30, + 0x78, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x22, 0x2c, 0x22, 0x63, 0x6f, 0x69, 0x6e, + 0x62, 0x61, 0x73, 0x65, 0x22, 0x3a, 0x22, 0x30, + 0x78, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x22, 0x2c, 0x22, 0x61, 0x6c, 0x6c, 0x6f, + 0x63, 0x22, 0x3a, 0x7b, 0x22, 0x35, 0x37, 0x32, + 0x66, 0x34, 0x64, 0x38, 0x30, 0x66, 0x31, 0x30, + 0x66, 0x36, 0x36, 0x33, 0x62, 0x35, 0x30, 0x34, + 0x39, 0x66, 0x37, 0x38, 0x39, 0x35, 0x34, 0x36, + 0x66, 0x32, 0x35, 0x66, 0x37, 0x30, 0x62, 0x62, + 0x36, 0x32, 0x61, 0x37, 0x66, 0x22, 0x3a, 0x7b, + 0x22, 0x62, 0x61, 0x6c, 0x61, 0x6e, 0x63, 0x65, + 0x22, 0x3a, 0x22, 0x30, 0x78, 0x33, 0x33, 0x62, + 0x32, 0x65, 0x33, 0x63, 0x39, 0x66, 0x64, 0x30, + 0x38, 0x30, 0x34, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x22, 0x7d, 0x7d, 0x2c, + 0x22, 0x6e, 0x75, 0x6d, 0x62, 0x65, 0x72, 0x22, + 0x3a, 0x22, 0x30, 0x78, 0x30, 0x22, 0x2c, 0x22, + 0x67, 0x61, 0x73, 0x55, 0x73, 0x65, 0x64, 0x22, + 0x3a, 0x22, 0x30, 0x78, 0x30, 0x22, 0x2c, 0x22, + 0x70, 0x61, 0x72, 0x65, 0x6e, 0x74, 0x48, 0x61, + 0x73, 0x68, 0x22, 0x3a, 0x22, 0x30, 0x78, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, + 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x30, 0x22, + 0x7d, + }, + } CascadeConfig = Config{ MintAddresses: []string{ "95YUFjhDG892VePMzpwKF9JzewGKvGRi3", @@ -277,6 +393,8 @@ var ( // GetConfig ... func GetConfig(networkID uint32) *Config { switch networkID { + case DenaliID: + return &DenaliConfig case CascadeID: return &CascadeConfig default: diff --git a/genesis/genesis_test.go b/genesis/genesis_test.go index 825d07b15db..63742a7cf52 100644 --- a/genesis/genesis_test.go +++ b/genesis/genesis_test.go @@ -17,12 +17,15 @@ func TestNetworkName(t *testing.T) { if name := NetworkName(MainnetID); name != MainnetName { t.Fatalf("NetworkID was incorrectly named. Result: %s ; Expected: %s", name, MainnetName) } - if name := NetworkName(TestnetID); name != CascadeName { - t.Fatalf("NetworkID was incorrectly named. Result: %s ; Expected: %s", name, CascadeName) - } if name := NetworkName(CascadeID); name != CascadeName { t.Fatalf("NetworkID was incorrectly named. Result: %s ; Expected: %s", name, CascadeName) } + if name := NetworkName(DenaliID); name != DenaliName { + t.Fatalf("NetworkID was incorrectly named. Result: %s ; Expected: %s", name, DenaliName) + } + if name := NetworkName(TestnetID); name != DenaliName { + t.Fatalf("NetworkID was incorrectly named. Result: %s ; Expected: %s", name, DenaliName) + } if name := NetworkName(4294967295); name != "network-4294967295" { t.Fatalf("NetworkID was incorrectly named. Result: %s ; Expected: %s", name, "network-4294967295") } @@ -37,23 +40,39 @@ func TestNetworkID(t *testing.T) { t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", MainnetID, id) } - id, err = NetworkID(TestnetName) + id, err = NetworkID(CascadeName) if err != nil { t.Fatal(err) } - if id != TestnetID { - t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", TestnetID, id) + if id != CascadeID { + t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", CascadeID, id) } - id, err = NetworkID(CascadeName) + id, err = NetworkID("cAsCaDe") if err != nil { t.Fatal(err) } - if id != TestnetID { - t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", TestnetID, id) + if id != CascadeID { + t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", CascadeID, id) } - id, err = NetworkID("cAsCaDe") + id, err = NetworkID(DenaliName) + if err != nil { + t.Fatal(err) + } + if id != DenaliID { + t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", DenaliID, id) + } + + id, err = NetworkID("dEnAlI") + if err != nil { + t.Fatal(err) + } + if id != DenaliID { + t.Fatalf("Returned wrong network. Expected: %d ; Returned %d", DenaliID, id) + } + + id, err = NetworkID(TestnetName) if err != nil { t.Fatal(err) } diff --git a/genesis/network_id.go b/genesis/network_id.go index 5e5c0ddee57..7be79684aea 100644 --- a/genesis/network_id.go +++ b/genesis/network_id.go @@ -14,24 +14,32 @@ import ( // Hardcoded network IDs var ( MainnetID uint32 = 1 - TestnetID uint32 = 2 CascadeID uint32 = 2 + DenaliID uint32 = 3 + + TestnetID uint32 = 3 LocalID uint32 = 12345 MainnetName = "mainnet" - TestnetName = "testnet" CascadeName = "cascade" + DenaliName = "denali" + + TestnetName = "testnet" LocalName = "local" NetworkIDToNetworkName = map[uint32]string{ MainnetID: MainnetName, - TestnetID: CascadeName, - LocalID: LocalName, + CascadeID: CascadeName, + DenaliID: DenaliName, + + LocalID: LocalName, } NetworkNameToNetworkID = map[string]uint32{ MainnetName: MainnetID, - TestnetName: TestnetID, CascadeName: CascadeID, + DenaliName: DenaliID, + + TestnetName: TestnetID, LocalName: LocalID, } diff --git a/main/params.go b/main/params.go index e0488b72ca4..b7da10d9f30 100644 --- a/main/params.go +++ b/main/params.go @@ -74,7 +74,7 @@ func init() { fs := flag.NewFlagSet("gecko", flag.ContinueOnError) // NetworkID: - networkName := fs.String("network-id", genesis.CascadeName, "Network ID this node will connect to") + networkName := fs.String("network-id", genesis.TestnetName, "Network ID this node will connect to") // Ava fees: fs.Uint64Var(&Config.AvaTxFee, "ava-tx-fee", 0, "Ava transaction fee, in $nAva") From f459113c9017ada9990b0b55cc787afadf05060d Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 15:53:26 -0400 Subject: [PATCH 19/29] added coreth 0.2 to go.sum --- go.sum | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/go.sum b/go.sum index f1d47222cff..c538b4b30e8 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,8 @@ github.com/aristanetworks/glog v0.0.0-20191112221043-67e8567f59f3/go.mod h1:KASm github.com/aristanetworks/goarista v0.0.0-20200520141224-0f14e646773f h1:uM6lu1fpmCwf54zb6Ckkvphioq8MLlyFb/TlTgPpCKc= github.com/aristanetworks/goarista v0.0.0-20200520141224-0f14e646773f/go.mod h1:QZe5Yh80Hp1b6JxQdpfSEEe8X7hTyTEZSosSrFf/oJE= github.com/aristanetworks/splunk-hec-go v0.3.3/go.mod h1:1VHO9r17b0K7WmOlLb9nTk/2YanvOEnLMUgsFrxBROc= -github.com/ava-labs/coreth v0.1.0 h1:Cx9dkhkQ7Bc7jD07OTbFJRlNTeHbSSLUwsCQlduTupg= -github.com/ava-labs/coreth v0.1.0/go.mod h1:pGolKipwq5vGIY2IBBcBkMYrqniXMsS5SBn+BBi4+Js= +github.com/ava-labs/coreth v0.2.0 h1:HjR4RMTnWvXhXlnEbFNGF5pbcxfemVxZeEzC4BTIrIw= +github.com/ava-labs/coreth v0.2.0/go.mod h1:pGolKipwq5vGIY2IBBcBkMYrqniXMsS5SBn+BBi4+Js= github.com/ava-labs/go-ethereum v1.9.3 h1:GmnMZ/dlvVAPFmWBzEpRJX49pUAymPfoASLNRJqR0AY= github.com/ava-labs/go-ethereum v1.9.3/go.mod h1:a+agc6fXfZFsPZCylA3ry4Y8CLCqLKg3Rc23NXZ9aw8= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= From 1abf68cee205669a28a4dca386bf798a12d327af Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 16:04:40 -0400 Subject: [PATCH 20/29] changed logs to returned errors --- vms/platformvm/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index 3ac8f7b58b2..9c9e7cbe44c 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -155,7 +155,7 @@ func (service *Service) GetCurrentValidators(_ *http.Request, args *GetCurrentVa case *addDefaultSubnetDelegatorTx: address = tx.Destination default: // Shouldn't happen - service.vm.Ctx.Log.Debug("unexpected type in currentValidators list") + return fmt.Errorf("couldn't get the destination address of %s", tx.ID()) } reply.Validators[i] = APIValidator{ @@ -215,7 +215,7 @@ func (service *Service) GetPendingValidators(_ *http.Request, args *GetPendingVa case *addDefaultSubnetDelegatorTx: address = tx.Destination default: // Shouldn't happen - service.vm.Ctx.Log.Debug("unexpected type in currentValidators list") + return fmt.Errorf("couldn't get the destination address of %s", tx.ID()) } reply.Validators[i] = APIValidator{ ID: vdr.ID(), From f24cdffc9044be3c08fdc1d65f055271fd4e8cd0 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 16:31:43 -0400 Subject: [PATCH 21/29] set up ips/ids for denali --- genesis/config.go | 10 +++++----- main/params.go | 8 ++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/genesis/config.go b/genesis/config.go index d9a8672b68e..563324cd9d6 100644 --- a/genesis/config.go +++ b/genesis/config.go @@ -67,11 +67,11 @@ var ( "6cesTteH62Y5mLoDBUASaBvCXuL2AthL", }, StakerIDs: []string{ - "NX4zVkuiRJZYe6Nzzav7GXN3TakUet3Co", - "CMsa8cMw4eib1Hb8GG4xiUKAq5eE1BwUX", - "DsMP6jLhi1MkDVc3qx9xx9AAZWx8e87Jd", - "N86eodVZja3GEyZJTo3DFUPGpxEEvjGHs", - "EkKeGSLUbHrrtuayBtbwgWDRUiAziC3ao", + "C8TBZhhPTr53AKcwkwzfa7cromxL7ndNw", + "ATr9svGb2ameq1S3xjHfUYAheVF9a6RCL", + "LcWZdhnqfdUfhP9txf4eynNDdoosPnSFo", + "MLGUdJKazgcCbzRjM192Eggw2fyHMdq8d", + "EKGHUF3N8VTYBT3tDrUf9DjoTbMeGjkbZ", }, EVMBytes: []byte{ 0x7b, 0x22, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, diff --git a/main/params.go b/main/params.go index b7da10d9f30..785f6501b36 100644 --- a/main/params.go +++ b/main/params.go @@ -48,6 +48,14 @@ var ( // GetIPs returns the default IPs for each network func GetIPs(networkID uint32) []string { switch networkID { + case genesis.DenaliID: + return []string{ + "3.133.117.223:21001", + "18.224.140.156:21001", + "3.133.83.66:21001", + "3.133.131.39:21001", + "18.188.121.35:21001", + } case genesis.CascadeID: return []string{ "3.227.207.132:21001", From db9155ab2aca55d9199c8dc66299011facec0218 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 16:47:56 -0400 Subject: [PATCH 22/29] only reset the reconnect timer if the connection is marked as connected --- network/network.go | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/network/network.go b/network/network.go index 471de3ac53c..0cbca0decd9 100644 --- a/network/network.go +++ b/network/network.go @@ -122,6 +122,7 @@ type network struct { closed bool disconnectedIPs map[string]struct{} connectedIPs map[string]struct{} + retryDelay map[string]time.Duration // TODO: bound the size of [myIPs] to avoid DoS. LRU caching would be ideal myIPs map[string]struct{} // set of IPs that resulted in my ID. peers map[[20]byte]*peer @@ -228,6 +229,7 @@ func NewNetwork( disconnectedIPs: make(map[string]struct{}), connectedIPs: make(map[string]struct{}), + retryDelay: make(map[string]time.Duration), myIPs: map[string]struct{}{ip.String(): struct{}{}}, peers: make(map[[20]byte]*peer), } @@ -673,14 +675,28 @@ func (n *network) gossip() { // the network is closed func (n *network) connectTo(ip utils.IPDesc) { str := ip.String() - delay := n.initialReconnectDelay + n.stateLock.Lock() + delay := n.retryDelay[str] + n.stateLock.Unlock() + for { + time.Sleep(delay) + + if delay == 0 { + delay = n.initialReconnectDelay + } else { + delay *= 2 + } + if delay > n.maxReconnectDelay { + delay = n.maxReconnectDelay + } + n.stateLock.Lock() _, isDisconnected := n.disconnectedIPs[str] _, isConnected := n.connectedIPs[str] _, isMyself := n.myIPs[str] closed := n.closed - + n.retryDelay[str] = delay n.stateLock.Unlock() if !isDisconnected || isConnected || isMyself || closed { @@ -701,12 +717,6 @@ func (n *network) connectTo(ip utils.IPDesc) { } n.log.Verbo("error attempting to connect to %s: %s. Reattempting in %s", ip, err, delay) - - time.Sleep(delay) - delay *= 2 - if delay > n.maxReconnectDelay { - delay = n.maxReconnectDelay - } } } @@ -744,6 +754,7 @@ func (n *network) upgrade(p *peer, upgrader Upgrader) error { defer n.stateLock.Unlock() if n.closed { + p.conn.Close() return nil } @@ -759,6 +770,7 @@ func (n *network) upgrade(p *peer, upgrader Upgrader) error { } str := p.ip.String() delete(n.disconnectedIPs, str) + delete(n.retryDelay, str) n.myIPs[str] = struct{}{} } p.conn.Close() @@ -767,7 +779,9 @@ func (n *network) upgrade(p *peer, upgrader Upgrader) error { if _, ok := n.peers[key]; ok { if !p.ip.IsZero() { - delete(n.disconnectedIPs, p.ip.String()) + str := p.ip.String() + delete(n.disconnectedIPs, str) + delete(n.retryDelay, str) } p.conn.Close() return nil @@ -805,6 +819,7 @@ func (n *network) connected(p *peer) { str := p.ip.String() delete(n.disconnectedIPs, str) + delete(n.retryDelay, str) n.connectedIPs[str] = struct{}{} } From 8a6efcdebef415b17c1c6cf70360caed61994bfb Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 16:59:01 -0400 Subject: [PATCH 23/29] removed weird unlocking behavior in the retry mechanism --- network/network.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/network/network.go b/network/network.go index 0cbca0decd9..5b7f419fdbb 100644 --- a/network/network.go +++ b/network/network.go @@ -696,8 +696,6 @@ func (n *network) connectTo(ip utils.IPDesc) { _, isConnected := n.connectedIPs[str] _, isMyself := n.myIPs[str] closed := n.closed - n.retryDelay[str] = delay - n.stateLock.Unlock() if !isDisconnected || isConnected || isMyself || closed { // If the IP was discovered by the peer connecting to us, we don't @@ -708,8 +706,12 @@ func (n *network) connectTo(ip utils.IPDesc) { // If the network was closed, we should stop attempting to connect // to the peer + + n.stateLock.Unlock() return } + n.retryDelay[str] = delay + n.stateLock.Unlock() err := n.attemptConnect(ip) if err == nil { From 218ff1136c4365b6e576bc0f16b3aac17ac5f1bf Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sat, 30 May 2020 18:46:19 -0400 Subject: [PATCH 24/29] Changed genesis staking keys --- genesis/config.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/genesis/config.go b/genesis/config.go index 563324cd9d6..7442ca00756 100644 --- a/genesis/config.go +++ b/genesis/config.go @@ -67,11 +67,11 @@ var ( "6cesTteH62Y5mLoDBUASaBvCXuL2AthL", }, StakerIDs: []string{ - "C8TBZhhPTr53AKcwkwzfa7cromxL7ndNw", - "ATr9svGb2ameq1S3xjHfUYAheVF9a6RCL", - "LcWZdhnqfdUfhP9txf4eynNDdoosPnSFo", - "MLGUdJKazgcCbzRjM192Eggw2fyHMdq8d", - "EKGHUF3N8VTYBT3tDrUf9DjoTbMeGjkbZ", + "LQwRLm4cbJ7T2kxcxp4uXCU5XD8DFrE1C", + "hArafGhY2HFTbwaaVh1CSCUCUCiJ2Vfb", + "2m38qc95mhHXtrhjyGbe7r2NhniqHHJRB", + "4QBwET5o8kUhvt9xArhir4d3R25CtmZho", + "NpagUxt6KQiwPch9Sd4osv8kD1TZnkjdk", }, EVMBytes: []byte{ 0x7b, 0x22, 0x63, 0x6f, 0x6e, 0x66, 0x69, 0x67, From 07656c7c233681f56b73de4ae9ddb71d9d0a43ec Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sun, 31 May 2020 00:03:55 -0400 Subject: [PATCH 25/29] Require for reissued transactions to be orphans --- snow/engine/avalanche/transitive.go | 7 +- snow/engine/avalanche/transitive_test.go | 107 +++++++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/snow/engine/avalanche/transitive.go b/snow/engine/avalanche/transitive.go index 15f659e2cd0..a2bd3a1f38d 100644 --- a/snow/engine/avalanche/transitive.go +++ b/snow/engine/avalanche/transitive.go @@ -369,6 +369,7 @@ func (t *Transitive) batch(txs []snowstorm.Tx, force, empty bool) error { issuedTxs := ids.Set{} consumed := ids.Set{} issued := false + orphans := t.Consensus.Orphans() for _, tx := range txs { inputs := tx.InputIDs() overlaps := consumed.Overlaps(inputs) @@ -380,8 +381,10 @@ func (t *Transitive) batch(txs []snowstorm.Tx, force, empty bool) error { overlaps = false } - // Force allows for a conflict to be issued - if txID := tx.ID(); !overlaps && !issuedTxs.Contains(txID) && (force || t.Consensus.IsVirtuous(tx)) && !tx.Status().Decided() { + if txID := tx.ID(); !overlaps && // should never allow conflicting txs in the same vertex + !issuedTxs.Contains(txID) && // shouldn't issue duplicated transactions to the same vertex + (force || t.Consensus.IsVirtuous(tx)) && // force allows for a conflict to be issued + (!t.Consensus.TxIssued(tx) || orphans.Contains(txID)) { // should only reissued orphaned txs batch = append(batch, tx) issuedTxs.Add(txID) consumed.Union(inputs) diff --git a/snow/engine/avalanche/transitive_test.go b/snow/engine/avalanche/transitive_test.go index 31b8af8a1b6..90bb3fa576c 100644 --- a/snow/engine/avalanche/transitive_test.go +++ b/snow/engine/avalanche/transitive_test.go @@ -2975,3 +2975,110 @@ func TestEngineAggressivePolling(t *testing.T) { t.Fatalf("should have issued one pull query") } } + +func TestEngineDuplicatedIssuance(t *testing.T) { + config := DefaultConfig() + config.Params.BatchSize = 1 + config.Params.BetaVirtuous = 5 + config.Params.BetaRogue = 5 + + sender := &common.SenderTest{} + sender.T = t + config.Sender = sender + + sender.Default(true) + sender.CantGetAcceptedFrontier = false + + vdr := validators.GenerateRandomValidator(1) + + vals := validators.NewSet() + config.Validators = vals + + vals.Add(vdr) + + st := &stateTest{t: t} + config.State = st + + st.Default(true) + + vm := &VMTest{} + vm.T = t + config.VM = vm + + vm.Default(true) + + gVtx := &Vtx{ + id: GenerateID(), + status: choices.Accepted, + } + mVtx := &Vtx{ + id: GenerateID(), + status: choices.Accepted, + } + + gTx := &TestTx{ + TestTx: snowstorm.TestTx{ + Identifier: GenerateID(), + Stat: choices.Accepted, + }, + } + + utxos := []ids.ID{GenerateID(), GenerateID()} + + tx := &TestTx{ + TestTx: snowstorm.TestTx{ + Identifier: GenerateID(), + Deps: []snowstorm.Tx{gTx}, + Stat: choices.Processing, + }, + } + tx.Ins.Add(utxos[0]) + + st.edge = func() []ids.ID { return []ids.ID{gVtx.ID(), mVtx.ID()} } + st.getVertex = func(id ids.ID) (avalanche.Vertex, error) { + switch { + case id.Equals(gVtx.ID()): + return gVtx, nil + case id.Equals(mVtx.ID()): + return mVtx, nil + } + t.Fatalf("Unknown vertex") + panic("Should have errored") + } + + te := &Transitive{} + te.Initialize(config) + te.finishBootstrapping() + + lastVtx := new(Vtx) + st.buildVertex = func(_ ids.Set, txs []snowstorm.Tx) (avalanche.Vertex, error) { + consumers := []snowstorm.Tx{} + for _, tx := range txs { + consumers = append(consumers, tx) + } + lastVtx = &Vtx{ + parents: []avalanche.Vertex{gVtx, mVtx}, + id: GenerateID(), + txs: consumers, + status: choices.Processing, + bytes: []byte{1}, + } + return lastVtx, nil + } + + sender.CantPushQuery = false + + vm.PendingTxsF = func() []snowstorm.Tx { return []snowstorm.Tx{tx} } + te.Notify(common.PendingTxs) + + if len(lastVtx.txs) != 1 || !lastVtx.txs[0].ID().Equals(tx.ID()) { + t.Fatalf("Should have issued txs differently") + } + + st.buildVertex = func(ids.Set, []snowstorm.Tx) (avalanche.Vertex, error) { + t.Fatalf("shouldn't have attempted to issue a duplicated tx") + return nil, nil + } + + te.Notify(common.PendingTxs) +} From f6244aaca4187ab45295d78649eddbd1534419d4 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sun, 31 May 2020 15:43:01 -0400 Subject: [PATCH 26/29] allowed for multiple DB types to be used in the fund indexer --- vms/avm/export_tx_test.go | 11 +++++++++++ vms/components/ava/state.go | 6 +++--- vms/platformvm/service.go | 5 +++++ vms/platformvm/vm.go | 5 ++++- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/vms/avm/export_tx_test.go b/vms/avm/export_tx_test.go index 4c27d5811c5..67de4f214be 100644 --- a/vms/avm/export_tx_test.go +++ b/vms/avm/export_tx_test.go @@ -13,6 +13,7 @@ import ( "github.com/ava-labs/gecko/snow" "github.com/ava-labs/gecko/snow/engine/common" "github.com/ava-labs/gecko/utils/crypto" + "github.com/ava-labs/gecko/utils/hashing" "github.com/ava-labs/gecko/utils/logging" "github.com/ava-labs/gecko/vms/components/ava" "github.com/ava-labs/gecko/vms/components/codec" @@ -245,6 +246,16 @@ func TestIssueExportTx(t *testing.T) { if _, err := state.AVMUTXO(utxoID); err != nil { t.Fatal(err) } + + addrID := ids.NewID(hashing.ComputeHash256Array(key.PublicKey().Address().Bytes())) + + utxoIDs, err := state.AVMFunds(addrID) + if err != nil { + t.Fatal(err) + } + if len(utxoIDs) != 1 { + t.Fatalf("wrong number of utxoIDs %d", len(utxoIDs)) + } } // Test force accepting an import transaction. diff --git a/vms/components/ava/state.go b/vms/components/ava/state.go index 671cd93728d..fc3b929f8b7 100644 --- a/vms/components/ava/state.go +++ b/vms/components/ava/state.go @@ -119,7 +119,7 @@ func (s *State) SetStatus(id ids.ID, status choices.Status) error { // IDs returns a slice of IDs from storage func (s *State) IDs(id ids.ID) ([]ids.ID, error) { idSlice := []ids.ID(nil) - iter := prefixdb.New(id.Bytes(), s.DB).NewIterator() + iter := prefixdb.NewNested(id.Bytes(), s.DB).NewIterator() defer iter.Release() for iter.Next() { @@ -138,7 +138,7 @@ func (s *State) AddID(id ids.ID, key ids.ID) error { if key.IsZero() { return errZeroID } - db := prefixdb.New(id.Bytes(), s.DB) + db := prefixdb.NewNested(id.Bytes(), s.DB) return db.Put(key.Bytes(), nil) } @@ -147,6 +147,6 @@ func (s *State) RemoveID(id ids.ID, key ids.ID) error { if key.IsZero() { return errZeroID } - db := prefixdb.New(id.Bytes(), s.DB) + db := prefixdb.NewNested(id.Bytes(), s.DB) return db.Delete(key.Bytes()) } diff --git a/vms/platformvm/service.go b/vms/platformvm/service.go index 9c9e7cbe44c..ff53dfe5356 100644 --- a/vms/platformvm/service.go +++ b/vms/platformvm/service.go @@ -37,6 +37,7 @@ var ( errDSCantValidate = errors.New("new blockchain can't be validated by default Subnet") errNilSigner = errors.New("nil ShortID 'signer' is not valid") errNilTo = errors.New("nil ShortID 'to' is not valid") + errNoFunds = errors.New("no spendable funds were found") ) // Service defines the API calls that can be made to the platform chain @@ -987,6 +988,10 @@ func (service *Service) ImportAVA(_ *http.Request, args *ImportAVAArgs, response keys = append(keys, signers) } + if amount == 0 { + return errNoFunds + } + ava.SortTransferableInputsWithSigners(ins, keys) // Create the transaction diff --git a/vms/platformvm/vm.go b/vms/platformvm/vm.go index 9416a195c7a..b6f59ec562a 100644 --- a/vms/platformvm/vm.go +++ b/vms/platformvm/vm.go @@ -837,7 +837,10 @@ func (vm *VM) GetAtomicUTXOs(addrs ids.Set) ([]*ava.UTXO, error) { utxoIDs := ids.Set{} for _, addr := range addrs.List() { - utxos, _ := state.AVMFunds(addr) + utxos, err := state.AVMFunds(addr) + if err != nil { + return nil, err + } utxoIDs.Add(utxos...) } From 3c48165ef9d504569721a02eb055ab3befa04dbc Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sun, 31 May 2020 16:37:51 -0400 Subject: [PATCH 27/29] attempt multiple plugin directory locations --- main/params.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/main/params.go b/main/params.go index 785f6501b36..0b7ff59b287 100644 --- a/main/params.go +++ b/main/params.go @@ -39,6 +39,12 @@ var ( defaultDbDir = os.ExpandEnv(filepath.Join("$HOME", ".gecko", "db")) defaultStakingKeyPath = os.ExpandEnv(filepath.Join("$HOME", ".gecko", "staking", "staker.key")) defaultStakingCertPath = os.ExpandEnv(filepath.Join("$HOME", ".gecko", "staking", "staker.crt")) + + defaultPluginDirs = []string{ + "./build/plugins", + "./plugins", + os.ExpandEnv(filepath.Join("$HOME", ".gecko", "plugins")), + } ) var ( @@ -118,7 +124,7 @@ func init() { fs.StringVar(&Config.StakingCertFile, "staking-tls-cert-file", defaultStakingCertPath, "TLS certificate for staking") // Plugins: - fs.StringVar(&Config.PluginDir, "plugin-dir", "./build/plugins", "Plugin directory for Ava VMs") + fs.StringVar(&Config.PluginDir, "plugin-dir", defaultPluginDirs[0], "Plugin directory for Ava VMs") // Logging: logsDir := fs.String("log-dir", "", "Logging directory for Ava") @@ -257,6 +263,16 @@ func init() { } } + // Plugins + if _, err := os.Stat(Config.PluginDir); os.IsNotExist(err) { + for _, dir := range defaultPluginDirs { + if _, err := os.Stat(dir); !os.IsNotExist(err) { + Config.PluginDir = dir + break + } + } + } + // Staking Config.StakingCertFile = os.ExpandEnv(Config.StakingCertFile) // parse any env variable Config.StakingKeyFile = os.ExpandEnv(Config.StakingKeyFile) From 0f18f514b7d33d484b092313b0a9fe2a1cdd0560 Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sun, 31 May 2020 17:17:35 -0400 Subject: [PATCH 28/29] updated bootstrap ip --- main/params.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main/params.go b/main/params.go index 0b7ff59b287..b84a95bd78e 100644 --- a/main/params.go +++ b/main/params.go @@ -56,7 +56,7 @@ func GetIPs(networkID uint32) []string { switch networkID { case genesis.DenaliID: return []string{ - "3.133.117.223:21001", + "3.20.56.211:21001", "18.224.140.156:21001", "3.133.83.66:21001", "3.133.131.39:21001", From d38405178b960a45d3434162360e83f9e5c6939c Mon Sep 17 00:00:00 2001 From: StephenButtolph Date: Sun, 31 May 2020 18:01:09 -0400 Subject: [PATCH 29/29] final version bump to denali --- main/params.go | 2 +- node/node.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/main/params.go b/main/params.go index b84a95bd78e..ffc38a26f9c 100644 --- a/main/params.go +++ b/main/params.go @@ -29,7 +29,7 @@ import ( ) const ( - dbVersion = "v0.4.0" + dbVersion = "v0.5.0" ) // Results of parsing the CLI diff --git a/node/node.go b/node/node.go index 837d0169cd5..1408fecbd7e 100644 --- a/node/node.go +++ b/node/node.go @@ -55,7 +55,7 @@ const ( var ( genesisHashKey = []byte("genesisID") - nodeVersion = version.NewDefaultVersion("avalanche", 0, 4, 0) + nodeVersion = version.NewDefaultVersion("avalanche", 0, 5, 0) versionParser = version.NewDefaultParser() )