From 68f0ace86d77faa90601ddad77f5631c66151467 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Tue, 8 Dec 2020 19:45:16 -0500 Subject: Add unit tests for atomic tx issuance --- go.mod | 2 +- go.sum | 4 +- plugin/evm/export_tx_test.go | 4 -- plugin/evm/import_tx_test.go | 8 ++- plugin/evm/vm_test.go | 125 ++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 134 insertions(+), 9 deletions(-) diff --git a/go.mod b/go.mod index 8222317..e496c2d 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.14 require ( github.com/VictoriaMetrics/fastcache v1.5.7 - github.com/ava-labs/avalanchego v1.0.6-rc.2 + github.com/ava-labs/avalanchego v1.1.0 github.com/davecgh/go-spew v1.1.1 github.com/deckarep/golang-set v1.7.1 github.com/edsrzf/mmap-go v1.0.0 diff --git a/go.sum b/go.sum index 78d2529..8e599d3 100644 --- a/go.sum +++ b/go.sum @@ -46,8 +46,8 @@ github.com/aristanetworks/goarista v0.0.0-20170210015632-ea17b1a17847/go.mod h1: github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= -github.com/ava-labs/avalanchego v1.0.6-rc.2 h1:71wo8rMgZ/eC49G4bVk36JWeZpjAhdLUdbQf9/gxCYg= -github.com/ava-labs/avalanchego v1.0.6-rc.2/go.mod h1:Q/I7LaMv2EYL8plNVRbcpBJsDk2py2XISfov0KK1MgU= +github.com/ava-labs/avalanchego v1.1.0 h1:kJuOXGI+kt/jXi1TVJ0F4z1CP68YoDfYUHb4HHpjGHU= +github.com/ava-labs/avalanchego v1.1.0/go.mod h1:Q/I7LaMv2EYL8plNVRbcpBJsDk2py2XISfov0KK1MgU= github.com/aws/aws-sdk-go v1.25.48/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo= github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q= github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8= diff --git a/plugin/evm/export_tx_test.go b/plugin/evm/export_tx_test.go index 6fdf3a2..c1ddcda 100644 --- a/plugin/evm/export_tx_test.go +++ b/plugin/evm/export_tx_test.go @@ -126,7 +126,3 @@ func TestExportTxVerify(t *testing.T) { t.Fatal("ExportTx should have failed verification due to invalid output") } } - -func TestExportTxSemanticVerify(t *testing.T) { - -} diff --git a/plugin/evm/import_tx_test.go b/plugin/evm/import_tx_test.go index 973802a..139aa4e 100644 --- a/plugin/evm/import_tx_test.go +++ b/plugin/evm/import_tx_test.go @@ -178,7 +178,7 @@ func TestImportTxSemanticVerify(t *testing.T) { Outs: []EVMOutput{evmOutput}, } - state, err := vm.chain.BlockState(vm.lastAccepted.ethBlock) + state, err := vm.chain.CurrentState() if err != nil { t.Fatalf("Failed to get last accepted stateDB due to: %s", err) } @@ -291,6 +291,12 @@ func TestImportTxSemanticVerify(t *testing.T) { func TestNewImportTx(t *testing.T) { _, vm, _, sharedMemory := GenesisVM(t, true) + defer func() { + if err := vm.Shutdown(); err != nil { + t.Fatal(err) + } + }() + importAmount := uint64(1000000) utxoID := avax.UTXOID{ TxID: ids.ID{ diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 0e9c102..ecdc991 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -13,10 +13,13 @@ import ( "github.com/ava-labs/avalanchego/database/prefixdb" "github.com/ava-labs/avalanchego/ids" "github.com/ava-labs/avalanchego/snow" + "github.com/ava-labs/avalanchego/snow/choices" engCommon "github.com/ava-labs/avalanchego/snow/engine/common" "github.com/ava-labs/avalanchego/utils/crypto" "github.com/ava-labs/avalanchego/utils/formatting" "github.com/ava-labs/avalanchego/utils/logging" + "github.com/ava-labs/avalanchego/vms/components/avax" + "github.com/ava-labs/avalanchego/vms/secp256k1fx" "github.com/ava-labs/coreth/core" "github.com/ethereum/go-ethereum/common" ) @@ -134,5 +137,125 @@ func GenesisVM(t *testing.T, finishBootstrapping bool) (chan engCommon.Message, } func TestVMGenesis(t *testing.T) { - _, _, _, _ = GenesisVM(t, true) + _, vm, _, _ := GenesisVM(t, true) + + defer func() { + if err := vm.Shutdown(); err != nil { + t.Fatal(err) + } + }() +} + +func TestIssueTxs(t *testing.T) { + issuer, vm, _, sharedMemory := GenesisVM(t, true) + + defer func() { + if err := vm.Shutdown(); err != nil { + t.Fatal(err) + } + }() + + importAmount := uint64(10000000) + utxoID := avax.UTXOID{ + TxID: ids.ID{ + 0x0f, 0x2f, 0x4f, 0x6f, 0x8e, 0xae, 0xce, 0xee, + 0x0d, 0x2d, 0x4d, 0x6d, 0x8c, 0xac, 0xcc, 0xec, + 0x0b, 0x2b, 0x4b, 0x6b, 0x8a, 0xaa, 0xca, 0xea, + 0x09, 0x29, 0x49, 0x69, 0x88, 0xa8, 0xc8, 0xe8, + }, + } + + utxo := &avax.UTXO{ + UTXOID: utxoID, + Asset: avax.Asset{ID: vm.ctx.AVAXAssetID}, + Out: &secp256k1fx.TransferOutput{ + Amt: importAmount, + OutputOwners: secp256k1fx.OutputOwners{ + Threshold: 1, + Addrs: []ids.ShortID{testKeys[0].PublicKey().Address()}, + }, + }, + } + utxoBytes, err := vm.codec.Marshal(codecVersion, utxo) + if err != nil { + t.Fatal(err) + } + + xChainSharedMemory := sharedMemory.NewSharedMemory(vm.ctx.XChainID) + inputID := utxo.InputID() + if err := xChainSharedMemory.Put(vm.ctx.ChainID, []*atomic.Element{{ + Key: inputID[:], + Value: utxoBytes, + Traits: [][]byte{ + testKeys[0].PublicKey().Address().Bytes(), + }, + }}); err != nil { + t.Fatal(err) + } + + importTx, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[0], []*crypto.PrivateKeySECP256K1R{testKeys[0]}) + if err != nil { + t.Fatal(err) + } + + if err := vm.issueTx(importTx); err != nil { + t.Fatal(err) + } + + <-issuer + + blk, err := vm.BuildBlock() + if err != nil { + t.Fatal(err) + } + + if status := blk.Status(); status != choices.Processing { + t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status) + } + + if err := blk.Accept(); err != nil { + t.Fatal(err) + } + + if status := blk.Status(); status != choices.Accepted { + t.Fatalf("Expected status of accepted block to be %s, but found %s", choices.Accepted, status) + } + + lastAcceptedID := vm.LastAccepted() + if lastAcceptedID != blk.ID() { + t.Fatalf("Expected last accepted blockID to be the accepted block: %s, but found %s", blk.ID(), lastAcceptedID) + } + + exportTx, err := vm.newExportTx(vm.ctx.AVAXAssetID, importAmount-vm.txFee-1, vm.ctx.XChainID, testShortIDAddrs[0], []*crypto.PrivateKeySECP256K1R{testKeys[0]}) + if err != nil { + t.Fatal(err) + } + + if err := vm.issueTx(exportTx); err != nil { + t.Fatal(err) + } + + <-issuer + + blk2, err := vm.BuildBlock() + if err != nil { + t.Fatal(err) + } + + if status := blk2.Status(); status != choices.Processing { + t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status) + } + + if err := blk2.Accept(); err != nil { + t.Fatal(err) + } + + if status := blk2.Status(); status != choices.Accepted { + t.Fatalf("Expected status of accepted block to be %s, but found %s", choices.Accepted, status) + } + + lastAcceptedID = vm.LastAccepted() + if lastAcceptedID != blk2.ID() { + t.Fatalf("Expected last accepted blockID to be the accepted block: %s, but found %s", blk2.ID(), lastAcceptedID) + } } -- cgit v1.2.3-70-g09d2 From 0dd94e2a926b929c371d06c463f917a2c70d427d Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Fri, 11 Dec 2020 15:49:00 -0500 Subject: Improve miner handling of asynchronous block production --- miner/worker.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index 72597b0..2b44691 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -913,12 +913,15 @@ func (w *worker) commitNewWork(interrupt *int32, noempty bool, timestamp int64) //timestamp = int64(parent.Time() + 1) timestamp = int64(parent.Time()) } - // this will ensure we're not going off too far in the future - if now := time.Now().Unix(); timestamp > now+1 { - wait := time.Duration(timestamp-now) * time.Second - log.Info("Mining too far in the future", "wait", common.PrettyDuration(wait)) - time.Sleep(wait) - } + // This block has been removed to prevent BuildBlock + // from blocking until this chain advances to timestamp + // This prevents a block issued up to 30s in the future + // from causing an unnecessary sleep + // if now := time.Now().Unix(); timestamp > now+1 { + // wait := time.Duration(timestamp-now) * time.Second + // log.Info("Mining too far in the future", "wait", common.PrettyDuration(wait)) + // time.Sleep(wait) + // } num := parent.Number() header := &types.Header{ -- cgit v1.2.3-70-g09d2 From 96653ef8838a85a0dcf85484a54b77fa4c53a396 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Fri, 11 Dec 2020 17:31:05 -0500 Subject: Add Height to block interface --- plugin/evm/block.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugin/evm/block.go b/plugin/evm/block.go index 7c23c17..5a0d377 100644 --- a/plugin/evm/block.go +++ b/plugin/evm/block.go @@ -75,6 +75,11 @@ func (b *Block) Parent() snowman.Block { return &missing.Block{BlkID: parentID} } +// Height implements the snowman.Block interface +func (b *Block) Height() uint64 { + return b.ethBlock.Number().Uint64() +} + // Verify implements the snowman.Block interface func (b *Block) Verify() error { // Only enforce a minimum fee when bootstrapping has finished -- cgit v1.2.3-70-g09d2 From ddb43b6d824b5de77e4df9e9f551aa067be9b40e Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Fri, 11 Dec 2020 17:38:42 -0500 Subject: Update version --- plugin/evm/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin/evm/service.go b/plugin/evm/service.go index 2bb06df..e0b73eb 100644 --- a/plugin/evm/service.go +++ b/plugin/evm/service.go @@ -24,7 +24,7 @@ import ( ) const ( - version = "coreth-v0.3.15" + version = "coreth-v0.3.17" ) // test constants -- cgit v1.2.3-70-g09d2 From 3af0238046fe8d2f12dae0155087503f92375e34 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Mon, 14 Dec 2020 14:58:15 -0500 Subject: Add unit test for building conflicting blocks --- plugin/evm/vm.go | 27 ++++-- plugin/evm/vm_test.go | 264 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 283 insertions(+), 8 deletions(-) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 58ab600..5d379b7 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -156,9 +156,9 @@ type VM struct { acceptedDB database.Database + txPoolStabilizedLock sync.Mutex txPoolStabilizedHead common.Hash txPoolStabilizedOk chan struct{} - txPoolStabilizedLock sync.Mutex txPoolStabilizedShutdownChan chan struct{} metalock sync.Mutex @@ -401,7 +401,10 @@ func (vm *VM) Bootstrapping() error { return vm.fx.Bootstrapping() } // Bootstrapped notifies this VM that the consensus engine has finished // bootstrapping -func (vm *VM) Bootstrapped() error { return vm.fx.Bootstrapped() } +func (vm *VM) Bootstrapped() error { + vm.ctx.Bootstrapped() + return vm.fx.Bootstrapped() +} // Shutdown implements the snowman.ChainVM interface func (vm *VM) Shutdown() error { @@ -421,10 +424,10 @@ func (vm *VM) Shutdown() error { func (vm *VM) BuildBlock() (snowman.Block, error) { vm.chain.GenBlock() block := <-vm.newBlockChan - if block == nil { - return nil, errCreateBlock - } + // reset the min block time timer + // after finishing attempt to build + // a block. vm.bdlock.Lock() vm.bdTimerState = bdTimerStateMin vm.bdGenWaitFlag = false @@ -432,7 +435,11 @@ func (vm *VM) BuildBlock() (snowman.Block, error) { vm.blockDelayTimer.SetTimeoutIn(minBlockTime) vm.bdlock.Unlock() - log.Debug(fmt.Sprintf("built block %s", block.ID())) + if block == nil { + return nil, errCreateBlock + } + + log.Debug(fmt.Sprintf("Built block %s", block.ID())) // make sure Tx Pool is updated <-vm.txPoolStabilizedOk return block, nil @@ -739,7 +746,13 @@ func (vm *VM) awaitTxPoolStabilized() { defer vm.shutdownWg.Done() for { select { - case e := <-vm.newMinedBlockSub.Chan(): + case e, ok := <-vm.newMinedBlockSub.Chan(): + if !ok { + return + } + if e == nil { + continue + } switch h := e.Data.(type) { case core.NewMinedBlockEvent: vm.txPoolStabilizedLock.Lock() diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index ecdc991..4593404 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -4,10 +4,13 @@ package evm import ( + "crypto/rand" "encoding/json" + "math/big" "testing" "github.com/ava-labs/avalanchego/api/keystore" + "github.com/ava-labs/avalanchego/cache" "github.com/ava-labs/avalanchego/chains/atomic" "github.com/ava-labs/avalanchego/database/memdb" "github.com/ava-labs/avalanchego/database/prefixdb" @@ -20,7 +23,10 @@ import ( "github.com/ava-labs/avalanchego/utils/logging" "github.com/ava-labs/avalanchego/vms/components/avax" "github.com/ava-labs/avalanchego/vms/secp256k1fx" + "github.com/ava-labs/coreth" "github.com/ava-labs/coreth/core" + "github.com/ava-labs/coreth/core/types" + "github.com/ava-labs/coreth/params" "github.com/ethereum/go-ethereum/common" ) @@ -133,6 +139,16 @@ func GenesisVM(t *testing.T, finishBootstrapping bool) (chan engCommon.Message, t.Fatal(err) } + if finishBootstrapping { + if err := vm.Bootstrapping(); err != nil { + t.Fatal(err) + } + + if err := vm.Bootstrapped(); err != nil { + t.Fatal(err) + } + } + return issuer, vm, genesisBytes, m } @@ -146,7 +162,7 @@ func TestVMGenesis(t *testing.T) { }() } -func TestIssueTxs(t *testing.T) { +func TestIssueAtomicTxs(t *testing.T) { issuer, vm, _, sharedMemory := GenesisVM(t, true) defer func() { @@ -209,6 +225,10 @@ func TestIssueTxs(t *testing.T) { t.Fatal(err) } + if err := blk.Verify(); err != nil { + t.Fatal(err) + } + if status := blk.Status(); status != choices.Processing { t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status) } @@ -242,6 +262,10 @@ func TestIssueTxs(t *testing.T) { t.Fatal(err) } + if err := blk2.Verify(); err != nil { + t.Fatal(err) + } + if status := blk2.Status(); status != choices.Processing { t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status) } @@ -259,3 +283,241 @@ func TestIssueTxs(t *testing.T) { t.Fatalf("Expected last accepted blockID to be the accepted block: %s, but found %s", blk2.ID(), lastAcceptedID) } } + +func TestBuildEthTxBlock(t *testing.T) { + issuer, vm, _, sharedMemory := GenesisVM(t, true) + + defer func() { + if err := vm.Shutdown(); err != nil { + t.Fatal(err) + } + }() + + key, err := coreth.NewKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + + importAmount := uint64(10000000) + utxoID := avax.UTXOID{ + TxID: ids.ID{ + 0x0f, 0x2f, 0x4f, 0x6f, 0x8e, 0xae, 0xce, 0xee, + 0x0d, 0x2d, 0x4d, 0x6d, 0x8c, 0xac, 0xcc, 0xec, + 0x0b, 0x2b, 0x4b, 0x6b, 0x8a, 0xaa, 0xca, 0xea, + 0x09, 0x29, 0x49, 0x69, 0x88, 0xa8, 0xc8, 0xe8, + }, + } + + utxo := &avax.UTXO{ + UTXOID: utxoID, + Asset: avax.Asset{ID: vm.ctx.AVAXAssetID}, + Out: &secp256k1fx.TransferOutput{ + Amt: importAmount, + OutputOwners: secp256k1fx.OutputOwners{ + Threshold: 1, + Addrs: []ids.ShortID{testKeys[0].PublicKey().Address()}, + }, + }, + } + utxoBytes, err := vm.codec.Marshal(codecVersion, utxo) + if err != nil { + t.Fatal(err) + } + + xChainSharedMemory := sharedMemory.NewSharedMemory(vm.ctx.XChainID) + inputID := utxo.InputID() + if err := xChainSharedMemory.Put(vm.ctx.ChainID, []*atomic.Element{{ + Key: inputID[:], + Value: utxoBytes, + Traits: [][]byte{ + testKeys[0].PublicKey().Address().Bytes(), + }, + }}); err != nil { + t.Fatal(err) + } + + importTx, err := vm.newImportTx(vm.ctx.XChainID, key.Address, []*crypto.PrivateKeySECP256K1R{testKeys[0]}) + if err != nil { + t.Fatal(err) + } + + if err := vm.issueTx(importTx); err != nil { + t.Fatal(err) + } + + <-issuer + + blk, err := vm.BuildBlock() + if err != nil { + t.Fatal(err) + } + + if err := blk.Verify(); err != nil { + t.Fatal(err) + } + + if status := blk.Status(); status != choices.Processing { + t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status) + } + + if err := blk.Accept(); err != nil { + t.Fatal(err) + } + + txs := make([]*types.Transaction, 10) + for i := 0; i < 10; i++ { + tx := types.NewTransaction(uint64(i), key.Address, big.NewInt(10), 21000, params.MinGasPrice, nil) + signedTx, err := types.SignTx(tx, types.NewEIP155Signer(vm.chainID), key.PrivateKey) + if err != nil { + t.Fatal(err) + } + txs[i] = signedTx + } + errs := vm.chain.AddRemoteTxs(txs) + for i, err := range errs { + if err != nil { + t.Fatalf("Failed to add tx at index %d: %s", i, err) + } + } + + <-issuer + + blk, err = vm.BuildBlock() + if err != nil { + t.Fatal(err) + } + + if err := blk.Verify(); err != nil { + t.Fatal(err) + } + + if status := blk.Status(); status != choices.Processing { + t.Fatalf("Expected status of built block to be %s, but found %s", choices.Processing, status) + } + + if err := blk.Accept(); err != nil { + t.Fatal(err) + } + + if status := blk.Status(); status != choices.Accepted { + t.Fatalf("Expected status of accepted block to be %s, but found %s", choices.Accepted, status) + } + + lastAcceptedID := vm.LastAccepted() + if lastAcceptedID != blk.ID() { + t.Fatalf("Expected last accepted blockID to be the accepted block: %s, but found %s", blk.ID(), lastAcceptedID) + } +} + +func TestConflictingImportTxs(t *testing.T) { + issuer, vm, _, sharedMemory := GenesisVM(t, true) + + defer func() { + if err := vm.Shutdown(); err != nil { + t.Fatal(err) + } + }() + + conflictKey, err := coreth.NewKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + + xChainSharedMemory := sharedMemory.NewSharedMemory(vm.ctx.XChainID) + importTxs := make([]*Tx, 0, 3) + conflictTxs := make([]*Tx, 0, 3) + for i, key := range testKeys { + importAmount := uint64(10000000) + utxoID := avax.UTXOID{ + TxID: ids.ID{byte(i)}, + } + + utxo := &avax.UTXO{ + UTXOID: utxoID, + Asset: avax.Asset{ID: vm.ctx.AVAXAssetID}, + Out: &secp256k1fx.TransferOutput{ + Amt: importAmount, + OutputOwners: secp256k1fx.OutputOwners{ + Threshold: 1, + Addrs: []ids.ShortID{key.PublicKey().Address()}, + }, + }, + } + utxoBytes, err := vm.codec.Marshal(codecVersion, utxo) + if err != nil { + t.Fatal(err) + } + + inputID := utxo.InputID() + if err := xChainSharedMemory.Put(vm.ctx.ChainID, []*atomic.Element{{ + Key: inputID[:], + Value: utxoBytes, + Traits: [][]byte{ + key.PublicKey().Address().Bytes(), + }, + }}); err != nil { + t.Fatal(err) + } + + importTx, err := vm.newImportTx(vm.ctx.XChainID, testEthAddrs[i], []*crypto.PrivateKeySECP256K1R{key}) + if err != nil { + t.Fatal(err) + } + importTxs = append(importTxs, importTx) + + conflictTx, err := vm.newImportTx(vm.ctx.XChainID, conflictKey.Address, []*crypto.PrivateKeySECP256K1R{key}) + if err != nil { + t.Fatal(err) + } + conflictTxs = append(conflictTxs, conflictTx) + } + + expectedParentBlkID := vm.LastAccepted() + for i, tx := range importTxs { + if err := vm.issueTx(tx); err != nil { + t.Fatal(err) + } + + <-issuer + + blk, err := vm.BuildBlock() + if err != nil { + t.Fatal(err) + } + + if err := blk.Verify(); err != nil { + t.Fatal(err) + } + + if status := blk.Status(); status != choices.Processing { + t.Fatalf("Expected status of built block %d to be %s, but found %s", i, choices.Processing, status) + } + + if parentID := blk.Parent().ID(); parentID != expectedParentBlkID { + t.Fatalf("Expected parent to have blockID %s, but found %s", expectedParentBlkID, parentID) + } + + expectedParentBlkID = blk.ID() + vm.SetPreference(blk.ID()) + } + + // Shrink the atomic input cache to ensure that + // verification handles cache misses correctly. + vm.blockAtomicInputCache = cache.LRU{Size: 1} + + for i, tx := range conflictTxs { + if err := vm.issueTx(tx); err != nil { + t.Fatal(err) + } + + <-issuer + + _, err := vm.BuildBlock() + // The new block is verified in BuildBlock, so + // BuildBlock should fail due to an attempt to + // double spend an atomic UTXO. + if err == nil { + t.Fatalf("Block verification should have failed in BuildBlock %d due to double spending atomic UTXO", i) + } + } +} -- cgit v1.2.3-70-g09d2 From de9c3bc629f7e712b3cd84b280db6a32b9bc7030 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Mon, 14 Dec 2020 16:22:22 -0500 Subject: Fix linting and add to CI --- .ci/lint.sh | 12 + .github/workflows/ci.yml | 14 + .golangci.yml | 50 +++ accounts/keystore/account_cache.go | 2 +- consensus/dummy/consensus.go | 3 +- core/state/statedb.go | 1 - core/types/block.go | 2 +- core/vm/instructions.go | 1 + coreth.go | 4 - eth/backend.go | 25 +- eth/gasprice/gasprice.go | 2 - eth/protocol.go | 221 ----------- ethclient/ethclient.go | 14 +- ethstats/ethstats.go | 785 ------------------------------------- node/api.go | 1 - node/config.go | 2 +- node/node.go | 1 - notes/copied-list.txt | 3 - notes/hacked-list.txt | 2 + plugin/evm/error.go | 2 +- plugin/evm/service.go | 3 +- 21 files changed, 110 insertions(+), 1040 deletions(-) create mode 100755 .ci/lint.sh create mode 100644 .golangci.yml delete mode 100644 eth/protocol.go delete mode 100644 ethstats/ethstats.go diff --git a/.ci/lint.sh b/.ci/lint.sh new file mode 100755 index 0000000..cde7292 --- /dev/null +++ b/.ci/lint.sh @@ -0,0 +1,12 @@ +# binary will be $(go env GOPATH)/bin/golangci-lint +curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.30.0 +# or install it into ./bin/ +curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.30.0 +# In alpine linux (as it does not come with curl by default) +wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.30.0 + +export PATH=$PATH:$(go env GOPATH)/bin + +golangci-lint --version + +golangci-lint run --max-same-issues 0 \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31ae101..b371b2f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,20 @@ name: CI on: [pull_request, push] jobs: + lint: + name: Golang v${{ matrix.go }} (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + matrix: + go: ['1.15'] + os: [ubuntu-18.04] + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v1 + with: + go-version: ${{ matrix.go }} + - run: .ci/lint.sh + shell: bash test: name: Golang v${{ matrix.go }} (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..18b325e --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,50 @@ +# This file configures github.com/golangci/golangci-lint. + +run: + timeout: 3m + tests: true + # default is true. Enables skipping of directories: + # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ + skip-dirs-use-default: true + skip-files: + - core/genesis_alloc.go + +linters: + disable-all: true + enable: + - deadcode + - goconst + - goimports + - gosimple + - govet + - ineffassign + - misspell + # - staticcheck + - unconvert + # - unused + - varcheck + +linters-settings: + gofmt: + simplify: true + goconst: + min-len: 3 # minimum length of string constant + min-occurrences: 6 # minimum number of occurrences + +issues: + exclude-rules: + - path: crypto/blake2b/ + linters: + - deadcode + - path: crypto/bn256/cloudflare + linters: + - deadcode + - path: p2p/discv5/ + linters: + - deadcode + - path: core/vm/instructions_test.go + linters: + - goconst + - path: cmd/faucet/ + linters: + - deadcode diff --git a/accounts/keystore/account_cache.go b/accounts/keystore/account_cache.go index 76bd552..7ae4d37 100644 --- a/accounts/keystore/account_cache.go +++ b/accounts/keystore/account_cache.go @@ -28,9 +28,9 @@ import ( "time" "github.com/ava-labs/coreth/accounts" + mapset "github.com/deckarep/golang-set" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/log" - mapset "github.com/deckarep/golang-set" ) // Minimum amount of time between cache reloads. This limit applies if the platform does diff --git a/consensus/dummy/consensus.go b/consensus/dummy/consensus.go index da63673..866ff97 100644 --- a/consensus/dummy/consensus.go +++ b/consensus/dummy/consensus.go @@ -3,11 +3,12 @@ package dummy import ( "errors" "fmt" - "golang.org/x/crypto/sha3" "math/big" "runtime" "time" + "golang.org/x/crypto/sha3" + "github.com/ava-labs/coreth/consensus" "github.com/ava-labs/coreth/core/state" "github.com/ava-labs/coreth/core/types" diff --git a/core/state/statedb.go b/core/state/statedb.go index c4d926d..f75428b 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -43,7 +43,6 @@ type revision struct { var ( // emptyRoot is the known root hash of an empty trie. emptyRoot = common.HexToHash("56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421") - zeroRoot = common.HexToHash("0000000000000000000000000000000000000000000000000000000000000000") ) type proofList [][]byte diff --git a/core/types/block.go b/core/types/block.go index 8e23488..b293ae7 100644 --- a/core/types/block.go +++ b/core/types/block.go @@ -86,7 +86,7 @@ type Header struct { Extra []byte `json:"extraData" gencodec:"required"` MixDigest common.Hash `json:"mixHash"` Nonce BlockNonce `json:"nonce"` - ExtDataHash common.Hash `json:"extDataHash" gencodec:"required"` + ExtDataHash common.Hash `json:"extDataHash" gencodec:"required"` } // field type overrides for gencodec diff --git a/core/vm/instructions.go b/core/vm/instructions.go index e1eb25e..3e1da26 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -18,6 +18,7 @@ package vm import ( "errors" + "github.com/ava-labs/coreth/core/types" "github.com/ava-labs/coreth/params" "github.com/ethereum/go-ethereum/common" diff --git a/coreth.go b/coreth.go index 351a14d..25e9da2 100644 --- a/coreth.go +++ b/coreth.go @@ -40,10 +40,6 @@ type ETHChain struct { bcb *eth.BackendCallbacks } -func isLocalBlock(block *types.Block) bool { - return false -} - // NewETHChain creates an Ethereum blockchain with the given configs. func NewETHChain(config *eth.Config, nodecfg *node.Config, etherBase *common.Address, chainDB ethdb.Database) *ETHChain { if config == nil { diff --git a/eth/backend.go b/eth/backend.go index 728ec4d..9fb8f38 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -23,7 +23,6 @@ import ( "math/big" "runtime" "sync" - //"sync/atomic" "github.com/ava-labs/coreth/accounts" "github.com/ava-labs/coreth/consensus" @@ -53,6 +52,16 @@ import ( "github.com/ethereum/go-ethereum/rlp" ) +// ProtocolVersions are the supported versions of the eth protocol (first is primary). +var ProtocolVersions = []uint{eth65, eth64, eth63} + +// Constants to match up protocol versions and messages +const ( + eth63 = 63 + eth64 = 64 + eth65 = 65 +) + type BackendCallbacks struct { OnQueryAcceptedBlock func() *types.Block } @@ -498,13 +507,13 @@ func (s *Ethereum) Start() error { s.startBloomHandlers(params.BloomBitsBlocks) // Figure out a max peers count based on the server limits - maxPeers := s.p2pServer.MaxPeers - if s.config.LightServ > 0 { - if s.config.LightPeers >= s.p2pServer.MaxPeers { - return fmt.Errorf("invalid peer config: light peer count (%d) >= total peer count (%d)", s.config.LightPeers, s.p2pServer.MaxPeers) - } - maxPeers -= s.config.LightPeers - } + // maxPeers := s.p2pServer.MaxPeers + // if s.config.LightServ > 0 { + // if s.config.LightPeers >= s.p2pServer.MaxPeers { + // return fmt.Errorf("invalid peer config: light peer count (%d) >= total peer count (%d)", s.config.LightPeers, s.p2pServer.MaxPeers) + // } + // maxPeers -= s.config.LightPeers + // } // Start the networking layer and the light server if requested //s.protocolManager.Start(maxPeers) return nil diff --git a/eth/gasprice/gasprice.go b/eth/gasprice/gasprice.go index 14476ab..33810b2 100644 --- a/eth/gasprice/gasprice.go +++ b/eth/gasprice/gasprice.go @@ -29,8 +29,6 @@ import ( "github.com/ethereum/go-ethereum/log" ) -const sampleNumber = 3 // Number of transactions sampled in a block - var DefaultMaxPrice = big.NewInt(500 * params.GWei) type Config struct { diff --git a/eth/protocol.go b/eth/protocol.go deleted file mode 100644 index ef5dcde..0000000 --- a/eth/protocol.go +++ /dev/null @@ -1,221 +0,0 @@ -// Copyright 2014 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -package eth - -import ( - "fmt" - "io" - "math/big" - - "github.com/ava-labs/coreth/core" - "github.com/ava-labs/coreth/core/forkid" - "github.com/ava-labs/coreth/core/types" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/event" - "github.com/ethereum/go-ethereum/rlp" -) - -// Constants to match up protocol versions and messages -const ( - eth63 = 63 - eth64 = 64 - eth65 = 65 -) - -// protocolName is the official short name of the protocol used during capability negotiation. -const protocolName = "eth" - -// ProtocolVersions are the supported versions of the eth protocol (first is primary). -var ProtocolVersions = []uint{eth65, eth64, eth63} - -// protocolLengths are the number of implemented message corresponding to different protocol versions. -var protocolLengths = map[uint]uint64{eth65: 17, eth64: 17, eth63: 17} - -const protocolMaxMsgSize = 10 * 1024 * 1024 // Maximum cap on the size of a protocol message - -// eth protocol message codes -const ( - StatusMsg = 0x00 - NewBlockHashesMsg = 0x01 - TransactionMsg = 0x02 - GetBlockHeadersMsg = 0x03 - BlockHeadersMsg = 0x04 - GetBlockBodiesMsg = 0x05 - BlockBodiesMsg = 0x06 - NewBlockMsg = 0x07 - GetNodeDataMsg = 0x0d - NodeDataMsg = 0x0e - GetReceiptsMsg = 0x0f - ReceiptsMsg = 0x10 - - // New protocol message codes introduced in eth65 - // - // Previously these message ids were used by some legacy and unsupported - // eth protocols, reown them here. - NewPooledTransactionHashesMsg = 0x08 - GetPooledTransactionsMsg = 0x09 - PooledTransactionsMsg = 0x0a -) - -type errCode int - -const ( - ErrMsgTooLarge = iota - ErrDecode - ErrInvalidMsgCode - ErrProtocolVersionMismatch - ErrNetworkIDMismatch - ErrGenesisMismatch - ErrForkIDRejected - ErrNoStatusMsg - ErrExtraStatusMsg -) - -func (e errCode) String() string { - return errorToString[int(e)] -} - -// XXX change once legacy code is out -var errorToString = map[int]string{ - ErrMsgTooLarge: "Message too long", - ErrDecode: "Invalid message", - ErrInvalidMsgCode: "Invalid message code", - ErrProtocolVersionMismatch: "Protocol version mismatch", - ErrNetworkIDMismatch: "Network ID mismatch", - ErrGenesisMismatch: "Genesis mismatch", - ErrForkIDRejected: "Fork ID rejected", - ErrNoStatusMsg: "No status message", - ErrExtraStatusMsg: "Extra status message", -} - -type txPool interface { - // Has returns an indicator whether txpool has a transaction - // cached with the given hash. - Has(hash common.Hash) bool - - // Get retrieves the transaction from local txpool with given - // tx hash. - Get(hash common.Hash) *types.Transaction - - // AddRemotes should add the given transactions to the pool. - AddRemotes([]*types.Transaction) []error - - // Pending should return pending transactions. - // The slice should be modifiable by the caller. - Pending() (map[common.Address]types.Transactions, error) - - // SubscribeNewTxsEvent should return an event subscription of - // NewTxsEvent and send events to the given channel. - SubscribeNewTxsEvent(chan<- core.NewTxsEvent) event.Subscription -} - -// statusData63 is the network packet for the status message for eth/63. -type statusData63 struct { - ProtocolVersion uint32 - NetworkId uint64 - TD *big.Int - CurrentBlock common.Hash - GenesisBlock common.Hash -} - -// statusData is the network packet for the status message for eth/64 and later. -type statusData struct { - ProtocolVersion uint32 - NetworkID uint64 - TD *big.Int - Head common.Hash - Genesis common.Hash - ForkID forkid.ID -} - -// newBlockHashesData is the network packet for the block announcements. -type newBlockHashesData []struct { - Hash common.Hash // Hash of one particular block being announced - Number uint64 // Number of one particular block being announced -} - -// getBlockHeadersData represents a block header query. -type getBlockHeadersData struct { - Origin hashOrNumber // Block from which to retrieve headers - Amount uint64 // Maximum number of headers to retrieve - Skip uint64 // Blocks to skip between consecutive headers - Reverse bool // Query direction (false = rising towards latest, true = falling towards genesis) -} - -// hashOrNumber is a combined field for specifying an origin block. -type hashOrNumber struct { - Hash common.Hash // Block hash from which to retrieve headers (excludes Number) - Number uint64 // Block hash from which to retrieve headers (excludes Hash) -} - -// EncodeRLP is a specialized encoder for hashOrNumber to encode only one of the -// two contained union fields. -func (hn *hashOrNumber) EncodeRLP(w io.Writer) error { - if hn.Hash == (common.Hash{}) { - return rlp.Encode(w, hn.Number) - } - if hn.Number != 0 { - return fmt.Errorf("both origin hash (%x) and number (%d) provided", hn.Hash, hn.Number) - } - return rlp.Encode(w, hn.Hash) -} - -// DecodeRLP is a specialized decoder for hashOrNumber to decode the contents -// into either a block hash or a block number. -func (hn *hashOrNumber) DecodeRLP(s *rlp.Stream) error { - _, size, _ := s.Kind() - origin, err := s.Raw() - if err == nil { - switch { - case size == 32: - err = rlp.DecodeBytes(origin, &hn.Hash) - case size <= 8: - err = rlp.DecodeBytes(origin, &hn.Number) - default: - err = fmt.Errorf("invalid input size %d for origin", size) - } - } - return err -} - -// newBlockData is the network packet for the block propagation message. -type newBlockData struct { - Block *types.Block - TD *big.Int -} - -// sanityCheck verifies that the values are reasonable, as a DoS protection -func (request *newBlockData) sanityCheck() error { - if err := request.Block.SanityCheck(); err != nil { - return err - } - //TD at mainnet block #7753254 is 76 bits. If it becomes 100 million times - // larger, it will still fit within 100 bits - if tdlen := request.TD.BitLen(); tdlen > 100 { - return fmt.Errorf("too large block TD: bitlen %d", tdlen) - } - return nil -} - -// blockBody represents the data content of a single block. -type blockBody struct { - Transactions []*types.Transaction // Transactions contained within a block - Uncles []*types.Header // Uncles contained within a block -} - -// blockBodiesData is the network packet for block content distribution. -type blockBodiesData []*blockBody diff --git a/ethclient/ethclient.go b/ethclient/ethclient.go index 6dd4df8..8470d64 100644 --- a/ethclient/ethclient.go +++ b/ethclient/ethclient.go @@ -324,13 +324,13 @@ func toBlockNumArg(number *big.Int) string { return hexutil.EncodeBig(number) } -type rpcProgress struct { - StartingBlock hexutil.Uint64 - CurrentBlock hexutil.Uint64 - HighestBlock hexutil.Uint64 - PulledStates hexutil.Uint64 - KnownStates hexutil.Uint64 -} +// type rpcProgress struct { +// StartingBlock hexutil.Uint64 +// CurrentBlock hexutil.Uint64 +// HighestBlock hexutil.Uint64 +// PulledStates hexutil.Uint64 +// KnownStates hexutil.Uint64 +// } // SyncProgress retrieves the current progress of the sync algorithm. If there's // no sync currently running, it returns nil. diff --git a/ethstats/ethstats.go b/ethstats/ethstats.go deleted file mode 100644 index 3dd06e3..0000000 --- a/ethstats/ethstats.go +++ /dev/null @@ -1,785 +0,0 @@ -// Copyright 2016 The go-ethereum Authors -// This file is part of the go-ethereum library. -// -// The go-ethereum library is free software: you can redistribute it and/or modify -// it under the terms of the GNU Lesser General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// The go-ethereum library is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU Lesser General Public License for more details. -// -// You should have received a copy of the GNU Lesser General Public License -// along with the go-ethereum library. If not, see . - -// Package ethstats implements the network stats reporting service. -package ethstats - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "math/big" - "net/http" - "regexp" - "runtime" - "strconv" - "strings" - "sync" - "time" - - "github.com/ava-labs/coreth/consensus" - "github.com/ava-labs/coreth/core" - "github.com/ava-labs/coreth/core/types" - "github.com/ava-labs/coreth/eth" - "github.com/ava-labs/coreth/miner" - "github.com/ava-labs/coreth/node" - "github.com/ava-labs/coreth/rpc" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/common/mclock" - "github.com/ethereum/go-ethereum/eth/downloader" - "github.com/ethereum/go-ethereum/event" - "github.com/ethereum/go-ethereum/les" - "github.com/ethereum/go-ethereum/log" - "github.com/ethereum/go-ethereum/p2p" - "github.com/gorilla/websocket" -) - -const ( - // historyUpdateRange is the number of blocks a node should report upon login or - // history request. - historyUpdateRange = 50 - - // txChanSize is the size of channel listening to NewTxsEvent. - // The number is referenced from the size of tx pool. - txChanSize = 4096 - // chainHeadChanSize is the size of channel listening to ChainHeadEvent. - chainHeadChanSize = 10 -) - -// backend encompasses the bare-minimum functionality needed for ethstats reporting -type backend interface { - SubscribeChainHeadEvent(ch chan<- core.ChainHeadEvent) event.Subscription - SubscribeNewTxsEvent(ch chan<- core.NewTxsEvent) event.Subscription - CurrentHeader() *types.Header - HeaderByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Header, error) - GetTd(ctx context.Context, hash common.Hash) *big.Int - Stats() (pending int, queued int) - Downloader() *downloader.Downloader -} - -// fullNodeBackend encompasses the functionality necessary for a full node -// reporting to ethstats -type fullNodeBackend interface { - backend - Miner() *miner.Miner - BlockByNumber(ctx context.Context, number rpc.BlockNumber) (*types.Block, error) - CurrentBlock() *types.Block - SuggestPrice(ctx context.Context) (*big.Int, error) -} - -// Service implements an Ethereum netstats reporting daemon that pushes local -// chain statistics up to a monitoring server. -type Service struct { - server *p2p.Server // Peer-to-peer server to retrieve networking infos - backend backend - engine consensus.Engine // Consensus engine to retrieve variadic block fields - - node string // Name of the node to display on the monitoring page - pass string // Password to authorize access to the monitoring page - host string // Remote address of the monitoring service - - pongCh chan struct{} // Pong notifications are fed into this channel - histCh chan []uint64 // History request block numbers are fed into this channel - -} - -// connWrapper is a wrapper to prevent concurrent-write or concurrent-read on the -// websocket. -// -// From Gorilla websocket docs: -// Connections support one concurrent reader and one concurrent writer. -// Applications are responsible for ensuring that no more than one goroutine calls the write methods -// - NextWriter, SetWriteDeadline, WriteMessage, WriteJSON, EnableWriteCompression, SetCompressionLevel -// concurrently and that no more than one goroutine calls the read methods -// - NextReader, SetReadDeadline, ReadMessage, ReadJSON, SetPongHandler, SetPingHandler -// concurrently. -// The Close and WriteControl methods can be called concurrently with all other methods. -type connWrapper struct { - conn *websocket.Conn - - rlock sync.Mutex - wlock sync.Mutex -} - -func newConnectionWrapper(conn *websocket.Conn) *connWrapper { - return &connWrapper{conn: conn} -} - -// WriteJSON wraps corresponding method on the websocket but is safe for concurrent calling -func (w *connWrapper) WriteJSON(v interface{}) error { - w.wlock.Lock() - defer w.wlock.Unlock() - - return w.conn.WriteJSON(v) -} - -// ReadJSON wraps corresponding method on the websocket but is safe for concurrent calling -func (w *connWrapper) ReadJSON(v interface{}) error { - w.rlock.Lock() - defer w.rlock.Unlock() - - return w.conn.ReadJSON(v) -} - -// Close wraps corresponding method on the websocket but is safe for concurrent calling -func (w *connWrapper) Close() error { - // The Close and WriteControl methods can be called concurrently with all other methods, - // so the mutex is not used here - return w.conn.Close() -} - -// New returns a monitoring service ready for stats reporting. -func New(node *node.Node, backend backend, engine consensus.Engine, url string) error { - // Parse the netstats connection url - re := regexp.MustCompile("([^:@]*)(:([^@]*))?@(.+)") - parts := re.FindStringSubmatch(url) - if len(parts) != 5 { - return fmt.Errorf("invalid netstats url: \"%s\", should be nodename:secret@host:port", url) - } - ethstats := &Service{ - backend: backend, - engine: engine, - server: node.Server(), - node: parts[1], - pass: parts[3], - host: parts[4], - pongCh: make(chan struct{}), - histCh: make(chan []uint64, 1), - } - - node.RegisterLifecycle(ethstats) - return nil -} - -// Start implements node.Lifecycle, starting up the monitoring and reporting daemon. -func (s *Service) Start() error { - go s.loop() - - log.Info("Stats daemon started") - return nil -} - -// Stop implements node.Lifecycle, terminating the monitoring and reporting daemon. -func (s *Service) Stop() error { - log.Info("Stats daemon stopped") - return nil -} - -// loop keeps trying to connect to the netstats server, reporting chain events -// until termination. -func (s *Service) loop() { - // Subscribe to chain events to execute updates on - chainHeadCh := make(chan core.ChainHeadEvent, chainHeadChanSize) - headSub := s.backend.SubscribeChainHeadEvent(chainHeadCh) - defer headSub.Unsubscribe() - - txEventCh := make(chan core.NewTxsEvent, txChanSize) - txSub := s.backend.SubscribeNewTxsEvent(txEventCh) - defer txSub.Unsubscribe() - - // Start a goroutine that exhausts the subscriptions to avoid events piling up - var ( - quitCh = make(chan struct{}) - headCh = make(chan *types.Block, 1) - txCh = make(chan struct{}, 1) - ) - go func() { - var lastTx mclock.AbsTime - - HandleLoop: - for { - select { - // Notify of chain head events, but drop if too frequent - case head := <-chainHeadCh: - select { - case headCh <- head.Block: - default: - } - - // Notify of new transaction events, but drop if too frequent - case <-txEventCh: - if time.Duration(mclock.Now()-lastTx) < time.Second { - continue - } - lastTx = mclock.Now() - - select { - case txCh <- struct{}{}: - default: - } - - // node stopped - case <-txSub.Err(): - break HandleLoop - case <-headSub.Err(): - break HandleLoop - } - } - close(quitCh) - }() - - // Resolve the URL, defaulting to TLS, but falling back to none too - path := fmt.Sprintf("%s/api", s.host) - urls := []string{path} - - // url.Parse and url.IsAbs is unsuitable (https://github.com/golang/go/issues/19779) - if !strings.Contains(path, "://") { - urls = []string{"wss://" + path, "ws://" + path} - } - - errTimer := time.NewTimer(0) - defer errTimer.Stop() - // Loop reporting until termination - for { - select { - case <-quitCh: - return - case <-errTimer.C: - // Establish a websocket connection to the server on any supported URL - var ( - conn *connWrapper - err error - ) - dialer := websocket.Dialer{HandshakeTimeout: 5 * time.Second} - header := make(http.Header) - header.Set("origin", "http://localhost") - for _, url := range urls { - c, _, e := dialer.Dial(url, header) - err = e - if err == nil { - conn = newConnectionWrapper(c) - break - } - } - if err != nil { - log.Warn("Stats server unreachable", "err", err) - errTimer.Reset(10 * time.Second) - continue - } - // Authenticate the client with the server - if err = s.login(conn); err != nil { - log.Warn("Stats login failed", "err", err) - conn.Close() - errTimer.Reset(10 * time.Second) - continue - } - go s.readLoop(conn) - - // Send the initial stats so our node looks decent from the get go - if err = s.report(conn); err != nil { - log.Warn("Initial stats report failed", "err", err) - conn.Close() - errTimer.Reset(0) - continue - } - // Keep sending status updates until the connection breaks - fullReport := time.NewTicker(15 * time.Second) - - for err == nil { - select { - case <-quitCh: - fullReport.Stop() - // Make sure the connection is closed - conn.Close() - return - - case <-fullReport.C: - if err = s.report(conn); err != nil { - log.Warn("Full stats report failed", "err", err) - } - case list := <-s.histCh: - if err = s.reportHistory(conn, list); err != nil { - log.Warn("Requested history report failed", "err", err) - } - case head := <-headCh: - if err = s.reportBlock(conn, head); err != nil { - log.Warn("Block stats report failed", "err", err) - } - if err = s.reportPending(conn); err != nil { - log.Warn("Post-block transaction stats report failed", "err", err) - } - case <-txCh: - if err = s.reportPending(conn); err != nil { - log.Warn("Transaction stats report failed", "err", err) - } - } - } - fullReport.Stop() - - // Close the current connection and establish a new one - conn.Close() - errTimer.Reset(0) - } - } -} - -// readLoop loops as long as the connection is alive and retrieves data packets -// from the network socket. If any of them match an active request, it forwards -// it, if they themselves are requests it initiates a reply, and lastly it drops -// unknown packets. -func (s *Service) readLoop(conn *connWrapper) { - // If the read loop exists, close the connection - defer conn.Close() - - for { - // Retrieve the next generic network packet and bail out on error - var blob json.RawMessage - if err := conn.ReadJSON(&blob); err != nil { - log.Warn("Failed to retrieve stats server message", "err", err) - return - } - // If the network packet is a system ping, respond to it directly - var ping string - if err := json.Unmarshal(blob, &ping); err == nil && strings.HasPrefix(ping, "primus::ping::") { - if err := conn.WriteJSON(strings.Replace(ping, "ping", "pong", -1)); err != nil { - log.Warn("Failed to respond to system ping message", "err", err) - return - } - continue - } - // Not a system ping, try to decode an actual state message - var msg map[string][]interface{} - if err := json.Unmarshal(blob, &msg); err != nil { - log.Warn("Failed to decode stats server message", "err", err) - return - } - log.Trace("Received message from stats server", "msg", msg) - if len(msg["emit"]) == 0 { - log.Warn("Stats server sent non-broadcast", "msg", msg) - return - } - command, ok := msg["emit"][0].(string) - if !ok { - log.Warn("Invalid stats server message type", "type", msg["emit"][0]) - return - } - // If the message is a ping reply, deliver (someone must be listening!) - if len(msg["emit"]) == 2 && command == "node-pong" { - select { - case s.pongCh <- struct{}{}: - // Pong delivered, continue listening - continue - default: - // Ping routine dead, abort - log.Warn("Stats server pinger seems to have died") - return - } - } - // If the message is a history request, forward to the event processor - if len(msg["emit"]) == 2 && command == "history" { - // Make sure the request is valid and doesn't crash us - request, ok := msg["emit"][1].(map[string]interface{}) - if !ok { - log.Warn("Invalid stats history request", "msg", msg["emit"][1]) - select { - case s.histCh <- nil: // Treat it as an no indexes request - default: - } - continue - } - list, ok := request["list"].([]interface{}) - if !ok { - log.Warn("Invalid stats history block list", "list", request["list"]) - return - } - // Convert the block number list to an integer list - numbers := make([]uint64, len(list)) - for i, num := range list { - n, ok := num.(float64) - if !ok { - log.Warn("Invalid stats history block number", "number", num) - return - } - numbers[i] = uint64(n) - } - select { - case s.histCh <- numbers: - continue - default: - } - } - // Report anything else and continue - log.Info("Unknown stats message", "msg", msg) - } -} - -// nodeInfo is the collection of meta information about a node that is displayed -// on the monitoring page. -type nodeInfo struct { - Name string `json:"name"` - Node string `json:"node"` - Port int `json:"port"` - Network string `json:"net"` - Protocol string `json:"protocol"` - API string `json:"api"` - Os string `json:"os"` - OsVer string `json:"os_v"` - Client string `json:"client"` - History bool `json:"canUpdateHistory"` -} - -// authMsg is the authentication infos needed to login to a monitoring server. -type authMsg struct { - ID string `json:"id"` - Info nodeInfo `json:"info"` - Secret string `json:"secret"` -} - -// login tries to authorize the client at the remote server. -func (s *Service) login(conn *connWrapper) error { - // Construct and send the login authentication - infos := s.server.NodeInfo() - - var network, protocol string - if info := infos.Protocols["eth"]; info != nil { - network = fmt.Sprintf("%d", info.(*eth.NodeInfo).Network) - protocol = fmt.Sprintf("eth/%d", eth.ProtocolVersions[0]) - } else { - network = fmt.Sprintf("%d", infos.Protocols["les"].(*les.NodeInfo).Network) - protocol = fmt.Sprintf("les/%d", les.ClientProtocolVersions[0]) - } - auth := &authMsg{ - ID: s.node, - Info: nodeInfo{ - Name: s.node, - Node: infos.Name, - Port: infos.Ports.Listener, - Network: network, - Protocol: protocol, - API: "No", - Os: runtime.GOOS, - OsVer: runtime.GOARCH, - Client: "0.1.1", - History: true, - }, - Secret: s.pass, - } - login := map[string][]interface{}{ - "emit": {"hello", auth}, - } - if err := conn.WriteJSON(login); err != nil { - return err - } - // Retrieve the remote ack or connection termination - var ack map[string][]string - if err := conn.ReadJSON(&ack); err != nil || len(ack["emit"]) != 1 || ack["emit"][0] != "ready" { - return errors.New("unauthorized") - } - return nil -} - -// report collects all possible data to report and send it to the stats server. -// This should only be used on reconnects or rarely to avoid overloading the -// server. Use the individual methods for reporting subscribed events. -func (s *Service) report(conn *connWrapper) error { - if err := s.reportLatency(conn); err != nil { - return err - } - if err := s.reportBlock(conn, nil); err != nil { - return err - } - if err := s.reportPending(conn); err != nil { - return err - } - if err := s.reportStats(conn); err != nil { - return err - } - return nil -} - -// reportLatency sends a ping request to the server, measures the RTT time and -// finally sends a latency update. -func (s *Service) reportLatency(conn *connWrapper) error { - // Send the current time to the ethstats server - start := time.Now() - - ping := map[string][]interface{}{ - "emit": {"node-ping", map[string]string{ - "id": s.node, - "clientTime": start.String(), - }}, - } - if err := conn.WriteJSON(ping); err != nil { - return err - } - // Wait for the pong request to arrive back - select { - case <-s.pongCh: - // Pong delivered, report the latency - case <-time.After(5 * time.Second): - // Ping timeout, abort - return errors.New("ping timed out") - } - latency := strconv.Itoa(int((time.Since(start) / time.Duration(2)).Nanoseconds() / 1000000)) - - // Send back the measured latency - log.Trace("Sending measured latency to ethstats", "latency", latency) - - stats := map[string][]interface{}{ - "emit": {"latency", map[string]string{ - "id": s.node, - "latency": latency, - }}, - } - return conn.WriteJSON(stats) -} - -// blockStats is the information to report about individual blocks. -type blockStats struct { - Number *big.Int `json:"number"` - Hash common.Hash `json:"hash"` - ParentHash common.Hash `json:"parentHash"` - Timestamp *big.Int `json:"timestamp"` - Miner common.Address `json:"miner"` - GasUsed uint64 `json:"gasUsed"` - GasLimit uint64 `json:"gasLimit"` - Diff string `json:"difficulty"` - TotalDiff string `json:"totalDifficulty"` - Txs []txStats `json:"transactions"` - TxHash common.Hash `json:"transactionsRoot"` - Root common.Hash `json:"stateRoot"` - Uncles uncleStats `json:"uncles"` -} - -// txStats is the information to report about individual transactions. -type txStats struct { - Hash common.Hash `json:"hash"` -} - -// uncleStats is a custom wrapper around an uncle array to force serializing -// empty arrays instead of returning null for them. -type uncleStats []*types.Header - -func (s uncleStats) MarshalJSON() ([]byte, error) { - if uncles := ([]*types.Header)(s); len(uncles) > 0 { - return json.Marshal(uncles) - } - return []byte("[]"), nil -} - -// reportBlock retrieves the current chain head and reports it to the stats server. -func (s *Service) reportBlock(conn *connWrapper, block *types.Block) error { - // Gather the block details from the header or block chain - details := s.assembleBlockStats(block) - - // Assemble the block report and send it to the server - log.Trace("Sending new block to ethstats", "number", details.Number, "hash", details.Hash) - - stats := map[string]interface{}{ - "id": s.node, - "block": details, - } - report := map[string][]interface{}{ - "emit": {"block", stats}, - } - return conn.WriteJSON(report) -} - -// assembleBlockStats retrieves any required metadata to report a single block -// and assembles the block stats. If block is nil, the current head is processed. -func (s *Service) assembleBlockStats(block *types.Block) *blockStats { - // Gather the block infos from the local blockchain - var ( - header *types.Header - td *big.Int - txs []txStats - uncles []*types.Header - ) - - // check if backend is a full node - fullBackend, ok := s.backend.(fullNodeBackend) - if ok { - if block == nil { - block = fullBackend.CurrentBlock() - } - header = block.Header() - td = fullBackend.GetTd(context.Background(), header.Hash()) - - txs = make([]txStats, len(block.Transactions())) - for i, tx := range block.Transactions() { - txs[i].Hash = tx.Hash() - } - uncles = block.Uncles() - } else { - // Light nodes would need on-demand lookups for transactions/uncles, skip - if block != nil { - header = block.Header() - } else { - header = s.backend.CurrentHeader() - } - td = s.backend.GetTd(context.Background(), header.Hash()) - txs = []txStats{} - } - - // Assemble and return the block stats - author, _ := s.engine.Author(header) - - return &blockStats{ - Number: header.Number, - Hash: header.Hash(), - ParentHash: header.ParentHash, - Timestamp: new(big.Int).SetUint64(header.Time), - Miner: author, - GasUsed: header.GasUsed, - GasLimit: header.GasLimit, - Diff: header.Difficulty.String(), - TotalDiff: td.String(), - Txs: txs, - TxHash: header.TxHash, - Root: header.Root, - Uncles: uncles, - } -} - -// reportHistory retrieves the most recent batch of blocks and reports it to the -// stats server. -func (s *Service) reportHistory(conn *connWrapper, list []uint64) error { - // Figure out the indexes that need reporting - indexes := make([]uint64, 0, historyUpdateRange) - if len(list) > 0 { - // Specific indexes requested, send them back in particular - indexes = append(indexes, list...) - } else { - // No indexes requested, send back the top ones - head := s.backend.CurrentHeader().Number.Int64() - start := head - historyUpdateRange + 1 - if start < 0 { - start = 0 - } - for i := uint64(start); i <= uint64(head); i++ { - indexes = append(indexes, i) - } - } - // Gather the batch of blocks to report - history := make([]*blockStats, len(indexes)) - for i, number := range indexes { - fullBackend, ok := s.backend.(fullNodeBackend) - // Retrieve the next block if it's known to us - var block *types.Block - if ok { - block, _ = fullBackend.BlockByNumber(context.Background(), rpc.BlockNumber(number)) // TODO ignore error here ? - } else { - if header, _ := s.backend.HeaderByNumber(context.Background(), rpc.BlockNumber(number)); header != nil { - block = types.NewBlockWithHeader(header) - } - } - // If we do have the block, add to the history and continue - if block != nil { - history[len(history)-1-i] = s.assembleBlockStats(block) - continue - } - // Ran out of blocks, cut the report short and send - history = history[len(history)-i:] - break - } - // Assemble the history report and send it to the server - if len(history) > 0 { - log.Trace("Sending historical blocks to ethstats", "first", history[0].Number, "last", history[len(history)-1].Number) - } else { - log.Trace("No history to send to stats server") - } - stats := map[string]interface{}{ - "id": s.node, - "history": history, - } - report := map[string][]interface{}{ - "emit": {"history", stats}, - } - return conn.WriteJSON(report) -} - -// pendStats is the information to report about pending transactions. -type pendStats struct { - Pending int `json:"pending"` -} - -// reportPending retrieves the current number of pending transactions and reports -// it to the stats server. -func (s *Service) reportPending(conn *connWrapper) error { - // Retrieve the pending count from the local blockchain - pending, _ := s.backend.Stats() - // Assemble the transaction stats and send it to the server - log.Trace("Sending pending transactions to ethstats", "count", pending) - - stats := map[string]interface{}{ - "id": s.node, - "stats": &pendStats{ - Pending: pending, - }, - } - report := map[string][]interface{}{ - "emit": {"pending", stats}, - } - return conn.WriteJSON(report) -} - -// nodeStats is the information to report about the local node. -type nodeStats struct { - Active bool `json:"active"` - Syncing bool `json:"syncing"` - Mining bool `json:"mining"` - Hashrate int `json:"hashrate"` - Peers int `json:"peers"` - GasPrice int `json:"gasPrice"` - Uptime int `json:"uptime"` -} - -// reportStats retrieves various stats about the node at the networking and -// mining layer and reports it to the stats server. -func (s *Service) reportStats(conn *connWrapper) error { - // Gather the syncing and mining infos from the local miner instance - var ( - mining bool - hashrate int - syncing bool - gasprice int - ) - // check if backend is a full node - fullBackend, ok := s.backend.(fullNodeBackend) - if ok { - mining = fullBackend.Miner().Mining() - hashrate = int(fullBackend.Miner().HashRate()) - - sync := fullBackend.Downloader().Progress() - syncing = fullBackend.CurrentHeader().Number.Uint64() >= sync.HighestBlock - - price, _ := fullBackend.SuggestPrice(context.Background()) - gasprice = int(price.Uint64()) - } else { - sync := s.backend.Downloader().Progress() - syncing = s.backend.CurrentHeader().Number.Uint64() >= sync.HighestBlock - } - // Assemble the node stats and send it to the server - log.Trace("Sending node details to ethstats") - - stats := map[string]interface{}{ - "id": s.node, - "stats": &nodeStats{ - Active: true, - Mining: mining, - Hashrate: hashrate, - Peers: s.server.PeerCount(), - GasPrice: gasprice, - Syncing: syncing, - Uptime: 100, - }, - } - report := map[string][]interface{}{ - "emit": {"stats", stats}, - } - return conn.WriteJSON(report) -} diff --git a/node/api.go b/node/api.go index 4589d25..af6ae69 100644 --- a/node/api.go +++ b/node/api.go @@ -19,7 +19,6 @@ package node import ( "context" "fmt" - //"strings" "github.com/ava-labs/coreth/internal/debug" "github.com/ava-labs/coreth/rpc" diff --git a/node/config.go b/node/config.go index 57bb7a1..73b59e7 100644 --- a/node/config.go +++ b/node/config.go @@ -30,7 +30,7 @@ import ( "github.com/ava-labs/coreth/accounts/external" "github.com/ava-labs/coreth/accounts/keystore" "github.com/ava-labs/coreth/accounts/scwallet" - //"github.com/ava-labs/coreth/accounts/usbwallet" + "github.com/ava-labs/coreth/rpc" "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/crypto" diff --git a/node/node.go b/node/node.go index e0a6424..341f213 100644 --- a/node/node.go +++ b/node/node.go @@ -55,7 +55,6 @@ type Node struct { const ( initializingState = iota - runningState closedState ) diff --git a/notes/copied-list.txt b/notes/copied-list.txt index 94353ff..e098f57 100644 --- a/notes/copied-list.txt +++ b/notes/copied-list.txt @@ -70,10 +70,7 @@ ./eth/filters/api.go ./eth/filters/filter.go ./eth/filters/filter_system.go -./eth/gasprice/gasprice.go ./eth/metrics.go -./eth/protocol.go -./ethstats/ethstats.go ./eth/tracers/internal/tracers/tracers.go ./eth/tracers/tracer.go ./eth/tracers/tracers.go diff --git a/notes/hacked-list.txt b/notes/hacked-list.txt index f4545ba..f951ffc 100644 --- a/notes/hacked-list.txt +++ b/notes/hacked-list.txt @@ -28,6 +28,7 @@ ./eth/api_tracer.go ./eth/backend.go ./eth/config.go +./eth/gasprice/gasprice.go ./eth/gen_config.go ./eth/tracers/internal/tracers/assets.go ./ethclient/ethclient.go @@ -44,3 +45,4 @@ ./rpc/client.go ./rpc/types.go ./interfaces.go +./accounts/keystore/account_cache.go (minor formatting change to pass linting) diff --git a/plugin/evm/error.go b/plugin/evm/error.go index 0554349..d65d28b 100644 --- a/plugin/evm/error.go +++ b/plugin/evm/error.go @@ -3,7 +3,7 @@ package evm -// TxError provides the ability for errors to be distinguished as permenant or +// TxError provides the ability for errors to be distinguished as permanent or // temporary type TxError interface { error diff --git a/plugin/evm/service.go b/plugin/evm/service.go index e0b73eb..8c9e16c 100644 --- a/plugin/evm/service.go +++ b/plugin/evm/service.go @@ -330,7 +330,6 @@ func (service *AvaxAPI) GetUTXOs(r *http.Request, args *api.GetUTXOsArgs, reply return fmt.Errorf("number of addresses given, %d, exceeds maximum, %d", len(args.Addresses), maxGetUTXOsAddrs) } - sourceChain := ids.ID{} if args.SourceChain == "" { return errNoSourceChain } @@ -339,7 +338,7 @@ func (service *AvaxAPI) GetUTXOs(r *http.Request, args *api.GetUTXOsArgs, reply if err != nil { return fmt.Errorf("problem parsing source chainID %q: %w", args.SourceChain, err) } - sourceChain = chainID + sourceChain := chainID addrSet := ids.ShortSet{} for _, addrStr := range args.Addresses { -- cgit v1.2.3-70-g09d2 From 1b6ea47fbf59f5e4569e88433f3ab85a45a1df75 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Tue, 15 Dec 2020 10:35:20 -0500 Subject: Simplify shutdown --- plugin/evm/vm.go | 22 ++++++++++------------ plugin/evm/vm_test.go | 21 +++++++++++++++++---- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 5d379b7..c9bdad9 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -156,10 +156,9 @@ type VM struct { acceptedDB database.Database - txPoolStabilizedLock sync.Mutex - txPoolStabilizedHead common.Hash - txPoolStabilizedOk chan struct{} - txPoolStabilizedShutdownChan chan struct{} + txPoolStabilizedLock sync.Mutex + txPoolStabilizedHead common.Hash + txPoolStabilizedOk chan struct{} metalock sync.Mutex blockCache, blockStatusCache cache.LRU @@ -175,7 +174,6 @@ type VM struct { genlock sync.Mutex txSubmitChan <-chan struct{} atomicTxSubmitChan chan struct{} - shutdownSubmitChan chan struct{} baseCodec codec.Codec codec codec.Manager clock timer.Clock @@ -183,7 +181,8 @@ type VM struct { pendingAtomicTxs chan *Tx blockAtomicInputCache cache.LRU - shutdownWg sync.WaitGroup + shutdownChan chan struct{} + shutdownWg sync.WaitGroup fx secp256k1fx.Fx } @@ -232,6 +231,7 @@ func (vm *VM) Initialize( return errUnsupportedFXs } + vm.shutdownChan = make(chan struct{}, 1) vm.ctx = ctx vm.chaindb = Database{db} g := new(core.Genesis) @@ -350,11 +350,9 @@ func (vm *VM) Initialize( vm.bdTimerState = bdTimerStateLong vm.bdGenWaitFlag = true vm.txPoolStabilizedOk = make(chan struct{}, 1) - vm.txPoolStabilizedShutdownChan = make(chan struct{}, 1) // Signal goroutine to shutdown // TODO: read size from options vm.pendingAtomicTxs = make(chan *Tx, 1024) vm.atomicTxSubmitChan = make(chan struct{}, 1) - vm.shutdownSubmitChan = make(chan struct{}, 1) vm.newMinedBlockSub = vm.chain.SubscribeNewMinedBlockEvent() vm.shutdownWg.Add(1) go ctx.Log.RecoverAndPanic(vm.awaitTxPoolStabilized) @@ -413,8 +411,8 @@ func (vm *VM) Shutdown() error { } vm.writeBackMetadata() - close(vm.txPoolStabilizedShutdownChan) - close(vm.shutdownSubmitChan) + vm.blockDelayTimer.Stop() + close(vm.shutdownChan) vm.chain.Stop() vm.shutdownWg.Wait() return nil @@ -763,7 +761,7 @@ func (vm *VM) awaitTxPoolStabilized() { vm.txPoolStabilizedLock.Unlock() default: } - case <-vm.txPoolStabilizedShutdownChan: + case <-vm.shutdownChan: return } } @@ -782,7 +780,7 @@ func (vm *VM) awaitSubmittedTxs() { vm.tryBlockGen() case <-time.After(5 * time.Second): vm.tryBlockGen() - case <-vm.shutdownSubmitChan: + case <-vm.shutdownChan: return } } diff --git a/plugin/evm/vm_test.go b/plugin/evm/vm_test.go index 4593404..dbceb1b 100644 --- a/plugin/evm/vm_test.go +++ b/plugin/evm/vm_test.go @@ -8,6 +8,7 @@ import ( "encoding/json" "math/big" "testing" + "time" "github.com/ava-labs/avalanchego/api/keystore" "github.com/ava-labs/avalanchego/cache" @@ -155,11 +156,23 @@ func GenesisVM(t *testing.T, finishBootstrapping bool) (chan engCommon.Message, func TestVMGenesis(t *testing.T) { _, vm, _, _ := GenesisVM(t, true) - defer func() { - if err := vm.Shutdown(); err != nil { - t.Fatal(err) + shutdownChan := make(chan error, 1) + shutdownFunc := func() { + err := vm.Shutdown() + shutdownChan <- err + } + + go shutdownFunc() + shutdownTimeout := 10 * time.Millisecond + ticker := time.NewTicker(shutdownTimeout) + select { + case <-ticker.C: + t.Fatalf("VM shutdown took longer than timeout: %v", shutdownTimeout) + case err := <-shutdownChan: + if err != nil { + t.Fatalf("Shutdown errored: %s", err) } - }() + } } func TestIssueAtomicTxs(t *testing.T) { -- cgit v1.2.3-70-g09d2 From b89114d6abd140dcd1fbf6e4e64fcba508a9f59c Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 17 Dec 2020 14:14:19 -0500 Subject: Address comments --- .ci/lint.sh | 2 +- .golangci.yml | 2 -- plugin/evm/vm.go | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.ci/lint.sh b/.ci/lint.sh index cde7292..e2ee0ce 100755 --- a/.ci/lint.sh +++ b/.ci/lint.sh @@ -9,4 +9,4 @@ export PATH=$PATH:$(go env GOPATH)/bin golangci-lint --version -golangci-lint run --max-same-issues 0 \ No newline at end of file +golangci-lint run --max-same-issues 0 diff --git a/.golangci.yml b/.golangci.yml index 18b325e..d2a5e38 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -19,9 +19,7 @@ linters: - govet - ineffassign - misspell - # - staticcheck - unconvert - # - unused - varcheck linters-settings: diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index c9bdad9..1caa9e9 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -739,7 +739,7 @@ func (vm *VM) writeBackMetadata() { // awaitTxPoolStabilized waits for a txPoolHead channel event // and notifies the VM when the tx pool has stabilized to the // expected block hash -// Waits for signal to shutdown from txPoolStabilizedShutdownChan chan +// Waits for signal to shutdown from [vm.shutdownChan] func (vm *VM) awaitTxPoolStabilized() { defer vm.shutdownWg.Done() for { -- cgit v1.2.3-70-g09d2 From 5f6acbf70cb34f8e7978ae5ef4b2c323b2605ff4 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 17 Dec 2020 14:14:45 -0500 Subject: Reduce overhead in ci lint script --- .ci/lint.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.ci/lint.sh b/.ci/lint.sh index e2ee0ce..e5363a8 100755 --- a/.ci/lint.sh +++ b/.ci/lint.sh @@ -1,9 +1,5 @@ # binary will be $(go env GOPATH)/bin/golangci-lint curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.30.0 -# or install it into ./bin/ -curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.30.0 -# In alpine linux (as it does not come with curl by default) -wget -O- -nv https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.30.0 export PATH=$PATH:$(go env GOPATH)/bin -- cgit v1.2.3-70-g09d2 From 4bfc175770fa006387ca1ffd2360fc0665dfcbf3 Mon Sep 17 00:00:00 2001 From: Aaron Buchwald Date: Thu, 17 Dec 2020 14:21:12 -0500 Subject: Change name of CI linting job --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b371b2f..c5ef50b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,7 +3,7 @@ on: [pull_request, push] jobs: lint: - name: Golang v${{ matrix.go }} (${{ matrix.os }}) + name: Lint Golang v${{ matrix.go }} (${{ matrix.os }}) runs-on: ${{ matrix.os }} strategy: matrix: -- cgit v1.2.3-70-g09d2 From 37f247ff4301958a42c15c6aeff68233907388de Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Thu, 17 Dec 2020 15:14:12 -0500 Subject: renamings; add comments --- plugin/evm/vm.go | 144 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 87 insertions(+), 57 deletions(-) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index c9bdad9..f125564 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -69,19 +69,15 @@ const ( maxBlockTime = 1000 * time.Millisecond batchSize = 250 maxUTXOsToFetch = 1024 - blockCacheSize = 1 << 10 // 1024 + blockCacheSize = 1024 codecVersion = uint16(0) ) const ( - bdTimerStateMin = iota - bdTimerStateMax - bdTimerStateLong + txFee = units.MilliAvax ) var ( - txFee = units.MilliAvax - errEmptyBlock = errors.New("empty block") errCreateBlock = errors.New("couldn't create block") errUnknownBlock = errors.New("unknown block") @@ -103,6 +99,17 @@ var ( errInvalidNonce = errors.New("invalid nonce") ) +// mayBuildBlockStatus denotes whether the engine should be notified +// that a block should be built, or whether more time has to pass +// before doing so. See VM's [mayBuildBlock]. +type mayBuildBlockStatus uint8 + +const ( + wait mayBuildBlockStatus = iota + conditionalWait + build +) + func maxDuration(x, y time.Duration) time.Duration { if x > y { return x @@ -145,14 +152,16 @@ type VM struct { CLIConfig CommandLineConfig - chainID *big.Int - networkID uint64 - genesisHash common.Hash - chain *coreth.ETHChain - chaindb Database - newBlockChan chan *Block - networkChan chan<- commonEng.Message - newMinedBlockSub *event.TypeMuxSubscription + chainID *big.Int + networkID uint64 + genesisHash common.Hash + chain *coreth.ETHChain + chaindb Database + newBlockChan chan *Block + // A message is sent on this channel when a new block + // is ready to be build. This notifies the consensus engine. + notifyBuildBlockChan chan<- commonEng.Message + newMinedBlockSub *event.TypeMuxSubscription acceptedDB database.Database @@ -165,11 +174,27 @@ type VM struct { lastAccepted *Block writingMetadata uint32 - bdlock sync.Mutex - blockDelayTimer *timer.Timer - bdTimerState int8 - bdGenWaitFlag bool - bdGenFlag bool + // [buildBlockLock] must be held when accessing [mayBuildBlock], + // [tryToBuildBlock] or [awaitingBuildBlock]. + buildBlockLock sync.Mutex + // [buildBlockTimer] periodically fires in order to update [mayBuildBlock] + // and to try to build a block, if applicable. + buildBlockTimer *timer.Timer + // [mayBuildBlock] == [wait] means that the next block may be built + // only after more time has elapsed. + // [mayBuildBlock] == [conditionalWait] means that the next block may be built + // only if it has more than [batchSize] txs in it. Otherwise, wait until more + // time has elapsed. + // [mayBuildBlock] == [build] means that the next block may be built + // at any time. + mayBuildBlock mayBuildBlockStatus + // If true, try to notify the engine that a block should be built. + // Engine may not be notified because [mayBuildBlock] says to wait. + tryToBuildBlock bool + // If true, the engine has been notified that it should build a block + // but has not done so yet. If this is the case, wait until it has + // built a block before notifying it again. + awaitingBuildBlock bool genlock sync.Mutex txSubmitChan <-chan struct{} @@ -329,26 +354,31 @@ func (vm *VM) Initialize( vm.blockStatusCache = cache.LRU{Size: blockCacheSize} vm.blockAtomicInputCache = cache.LRU{Size: blockCacheSize} vm.newBlockChan = make(chan *Block) - vm.networkChan = toEngine - vm.blockDelayTimer = timer.NewTimer(func() { - vm.bdlock.Lock() - switch vm.bdTimerState { - case bdTimerStateMin: - vm.bdTimerState = bdTimerStateMax - vm.blockDelayTimer.SetTimeoutIn(maxDuration(maxBlockTime-minBlockTime, 0)) - case bdTimerStateMax: - vm.bdTimerState = bdTimerStateLong + vm.notifyBuildBlockChan = toEngine + + // Periodically updates [vm.mayBuildBlock] and tries to notify the engine to build + // a new block, if applicable. + vm.buildBlockTimer = timer.NewTimer(func() { + vm.buildBlockLock.Lock() + switch vm.mayBuildBlock { + case wait: + // Some time has passed. Allow block to be built if it has enough txs in it. + vm.mayBuildBlock = conditionalWait + vm.buildBlockTimer.SetTimeoutIn(maxDuration(maxBlockTime-minBlockTime, 0)) + case conditionalWait: + // More time has passed. Allow block to be built regardless of tx count. + vm.mayBuildBlock = build } - tryAgain := vm.bdGenWaitFlag - vm.bdlock.Unlock() - if tryAgain { + tryBuildBlock := vm.tryToBuildBlock + vm.buildBlockLock.Unlock() + if tryBuildBlock { vm.tryBlockGen() } }) - go ctx.Log.RecoverAndPanic(vm.blockDelayTimer.Dispatch) + go ctx.Log.RecoverAndPanic(vm.buildBlockTimer.Dispatch) - vm.bdTimerState = bdTimerStateLong - vm.bdGenWaitFlag = true + vm.mayBuildBlock = build + vm.tryToBuildBlock = true vm.txPoolStabilizedOk = make(chan struct{}, 1) // TODO: read size from options vm.pendingAtomicTxs = make(chan *Tx, 1024) @@ -411,7 +441,7 @@ func (vm *VM) Shutdown() error { } vm.writeBackMetadata() - vm.blockDelayTimer.Stop() + vm.buildBlockTimer.Stop() close(vm.shutdownChan) vm.chain.Stop() vm.shutdownWg.Wait() @@ -423,15 +453,13 @@ func (vm *VM) BuildBlock() (snowman.Block, error) { vm.chain.GenBlock() block := <-vm.newBlockChan - // reset the min block time timer - // after finishing attempt to build - // a block. - vm.bdlock.Lock() - vm.bdTimerState = bdTimerStateMin - vm.bdGenWaitFlag = false - vm.bdGenFlag = false - vm.blockDelayTimer.SetTimeoutIn(minBlockTime) - vm.bdlock.Unlock() + vm.buildBlockLock.Lock() + // Specify that we should wait before trying to build another block. + vm.mayBuildBlock = wait + vm.tryToBuildBlock = false + vm.awaitingBuildBlock = false + vm.buildBlockTimer.SetTimeoutIn(minBlockTime) + vm.buildBlockLock.Unlock() if block == nil { return nil, errCreateBlock @@ -578,13 +606,14 @@ func (vm *VM) updateStatus(blockID ids.ID, status choices.Status) { } func (vm *VM) tryBlockGen() error { - vm.bdlock.Lock() - defer vm.bdlock.Unlock() - if vm.bdGenFlag { - // skip if one call already generates a block in this round + vm.buildBlockLock.Lock() + defer vm.buildBlockLock.Unlock() + if vm.awaitingBuildBlock { + // We notified the engine that a block should be built but it hasn't + // done so yet. Wait until it has done so before notifying again. return nil } - vm.bdGenWaitFlag = true + vm.tryToBuildBlock = true vm.genlock.Lock() defer vm.genlock.Unlock() @@ -597,20 +626,21 @@ func (vm *VM) tryBlockGen() error { return nil } - switch vm.bdTimerState { - case bdTimerStateMin: + switch vm.mayBuildBlock { + case wait: // Wait more time before notifying engine to building a block return nil - case bdTimerStateMax: + case conditionalWait: // Notify engine only if there are enough pending txs if size < batchSize { return nil } - case bdTimerStateLong: - // timeout; go ahead and generate a new block anyway + case build: // Notify engine + default: + panic(fmt.Sprintf("mayBuildBlock has unexpected value %d", vm.mayBuildBlock)) } select { - case vm.networkChan <- commonEng.PendingTxs: - // successfully push out the notification; this round ends - vm.bdGenFlag = true + case vm.notifyBuildBlockChan <- commonEng.PendingTxs: + // Notify engine to build a block + vm.awaitingBuildBlock = true default: return errBlockFrequency } -- cgit v1.2.3-70-g09d2 From f20fd5437ec5144a926589395cece29f2052994d Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Thu, 17 Dec 2020 16:53:56 -0500 Subject: rename mayBuildBlockStatus values --- plugin/evm/vm.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/plugin/evm/vm.go b/plugin/evm/vm.go index 231db2a..9335b51 100644 --- a/plugin/evm/vm.go +++ b/plugin/evm/vm.go @@ -105,9 +105,9 @@ var ( type mayBuildBlockStatus uint8 const ( - wait mayBuildBlockStatus = iota - conditionalWait - build + waitToBuild mayBuildBlockStatus = iota + conditionalWaitToBuild + mayBuild ) func maxDuration(x, y time.Duration) time.Duration { @@ -361,13 +361,13 @@ func (vm *VM) Initialize( vm.buildBlockTimer = timer.NewTimer(func() { vm.buildBlockLock.Lock() switch vm.mayBuildBlock { - case wait: + case waitToBuild: // Some time has passed. Allow block to be built if it has enough txs in it. - vm.mayBuildBlock = conditionalWait + vm.mayBuildBlock = conditionalWaitToBuild vm.buildBlockTimer.SetTimeoutIn(maxDuration(maxBlockTime-minBlockTime, 0)) - case conditionalWait: + case conditionalWaitToBuild: // More time has passed. Allow block to be built regardless of tx count. - vm.mayBuildBlock = build + vm.mayBuildBlock = mayBuild } tryBuildBlock := vm.tryToBuildBlock vm.buildBlockLock.Unlock() @@ -377,7 +377,7 @@ func (vm *VM) Initialize( }) go ctx.Log.RecoverAndPanic(vm.buildBlockTimer.Dispatch) - vm.mayBuildBlock = build + vm.mayBuildBlock = mayBuild vm.tryToBuildBlock = true vm.txPoolStabilizedOk = make(chan struct{}, 1) // TODO: read size from options @@ -455,7 +455,7 @@ func (vm *VM) BuildBlock() (snowman.Block, error) { vm.buildBlockLock.Lock() // Specify that we should wait before trying to build another block. - vm.mayBuildBlock = wait + vm.mayBuildBlock = waitToBuild vm.tryToBuildBlock = false vm.awaitingBuildBlock = false vm.buildBlockTimer.SetTimeoutIn(minBlockTime) @@ -627,13 +627,13 @@ func (vm *VM) tryBlockGen() error { } switch vm.mayBuildBlock { - case wait: // Wait more time before notifying engine to building a block + case waitToBuild: // Wait more time before notifying engine to building a block return nil - case conditionalWait: // Notify engine only if there are enough pending txs + case conditionalWaitToBuild: // Notify engine only if there are enough pending txs if size < batchSize { return nil } - case build: // Notify engine + case mayBuild: // Notify engine default: panic(fmt.Sprintf("mayBuildBlock has unexpected value %d", vm.mayBuildBlock)) } -- cgit v1.2.3-70-g09d2