diff --git a/.gometalinter.json b/.gometalinter.json new file mode 100644 index 0000000000..b1c7661b00 --- /dev/null +++ b/.gometalinter.json @@ -0,0 +1,11 @@ +{ + "Enable": [ + "gofmt", + "deadcode", + "misspell", + "goimports", + "nakedret", + "unparam", + "megacheck" + ] +} diff --git a/.travis.yml b/.travis.yml index 770ce2a4d4..3566c65579 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,7 +9,7 @@ matrix: - lint script: - - go get github.com/alecthomas/gometalinter && gometalinter --install && gometalinter + go get github.com/alecthomas/gometalinter && gometalinter --install && gometalinter ./... --deadline=10m - os: linux env: V=0.15.0 before_install: diff --git a/WORKSPACE b/WORKSPACE index c9ec57606e..379b08b474 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -65,3 +65,9 @@ go_repository( commit = "a49355c7e3f8fe157a85be2f77e6e269a0f89602", importpath = "golang.org/x/crypto", ) + +go_repository( + name = "com_github_syndtr_goleveldb", + commit = "c4c61651e9e37fa117f53c5a906d3b63090d8445", + importpath = "github.com/syndtr/goleveldb", +) diff --git a/beacon-chain/main.go b/beacon-chain/main.go index f5c320b7f3..446ff738ac 100644 --- a/beacon-chain/main.go +++ b/beacon-chain/main.go @@ -48,10 +48,7 @@ VERSION: app.Before = func(ctx *cli.Context) error { runtime.GOMAXPROCS(runtime.NumCPU()) - if err := debug.Setup(ctx); err != nil { - return err - } - return nil + return debug.Setup(ctx) } if err := app.Run(os.Args); err != nil { diff --git a/sharding/contracts/sharding_manager_test.go b/sharding/contracts/sharding_manager_test.go index 7ab17f092a..c7649d4f0b 100644 --- a/sharding/contracts/sharding_manager_test.go +++ b/sharding/contracts/sharding_manager_test.go @@ -32,7 +32,6 @@ var ( accountBalance2000Eth, _ = new(big.Int).SetString("2000000000000000000000", 10) notaryDepositInsufficient, _ = new(big.Int).SetString("999000000000000000000", 10) notaryDeposit, _ = new(big.Int).SetString("1000000000000000000000", 10) - FastForward100Blocks = 100 ctx = context.Background() ) @@ -163,6 +162,9 @@ func (s *smcTestHelper) addHeader(a *testAccount, shard *big.Int, period *big.In } cr, err := s.smc.CollationRecords(&bind.CallOpts{}, shard, period) + if err != nil { + return err + } if cr.ChunkRoot != [32]byte{chunkRoot} { return fmt.Errorf("Chunkroot mismatched. Want: %v, Got: %v", chunkRoot, cr) } @@ -349,6 +351,9 @@ func TestNotaryDeregisterThenRegister(t *testing.T) { // Notary 0 re-registers again. err = s.registerNotaries(notaryDeposit, 0, 1) + if err == nil { + t.Error("Expected re-registration to fail") + } err = checkNotaryPoolLength(s.smc, big.NewInt(0)) if err != nil { t.Errorf("Notary pool length mismatched: %v", err) @@ -407,22 +412,24 @@ func TestNotaryInstantRelease(t *testing.T) { s, _ := newSMCTestHelper(1) // Notary 0 registers. - err := s.registerNotaries(notaryDeposit, 0, 1) - err = checkNotaryPoolLength(s.smc, big.NewInt(1)) - if err != nil { + if err := s.registerNotaries(notaryDeposit, 0, 1); err != nil { + t.Error(err) + } + if err := checkNotaryPoolLength(s.smc, big.NewInt(1)); err != nil { t.Errorf("Notary pool length mismatched: %v", err) } s.fastForward(1) // Notary 0 deregisters. s.deregisterNotaries(0, 1) - err = checkNotaryPoolLength(s.smc, big.NewInt(0)) - if err != nil { + if err := checkNotaryPoolLength(s.smc, big.NewInt(0)); err != nil { t.Errorf("Notary pool length mismatched: %v", err) } // Notary 0 tries to release before lockup ends. - _, err = s.smc.ReleaseNotary(s.testAccounts[0].txOpts) + if _, err := s.smc.ReleaseNotary(s.testAccounts[0].txOpts); err == nil { + t.Error("Expected release notary to fail") + } s.backend.Commit() notary, err := s.smc.NotaryRegistry(&bind.CallOpts{}, s.testAccounts[0].addr) if err != nil { @@ -432,6 +439,9 @@ func TestNotaryInstantRelease(t *testing.T) { t.Errorf("Notary deposit flag should be true before released") } balance, err := s.backend.BalanceAt(ctx, s.testAccounts[0].addr, nil) + if err != nil { + t.Error(err) + } if balance.Cmp(notaryDeposit) > 0 { t.Errorf("Notary received deposit before lockup ends") } @@ -606,7 +616,9 @@ func TestAddHeadersAtWrongPeriod(t *testing.T) { func TestSubmitVote(t *testing.T) { s, _ := newSMCTestHelper(1) // Notary 0 registers. - err := s.registerNotaries(notaryDeposit, 0, 1) + if err := s.registerNotaries(notaryDeposit, 0, 1); err != nil { + t.Error(err) + } s.fastForward(1) // Proposer adds header consists shard 0, period 1 and chunkroot 0xA. @@ -614,8 +626,7 @@ func TestSubmitVote(t *testing.T) { shard0 := big.NewInt(0) index0 := big.NewInt(0) s.testAccounts[0].txOpts.Value = big.NewInt(0) - err = s.addHeader(&s.testAccounts[0], shard0, period1, 'A') - if err != nil { + if err := s.addHeader(&s.testAccounts[0], shard0, period1, 'A'); err != nil { t.Errorf("Proposer adds header failed: %v", err) } @@ -634,6 +645,9 @@ func TestSubmitVote(t *testing.T) { t.Fatalf("Notary submits vote failed: %v", err) } c, err = s.smc.GetVoteCount(&bind.CallOpts{}, shard0) + if err != nil { + t.Error(err) + } if c.Cmp(big.NewInt(1)) != 0 { t.Errorf("Incorrect notary vote count, want: 1, got: %v", c) } @@ -653,7 +667,9 @@ func TestSubmitVote(t *testing.T) { func TestSubmitVoteTwice(t *testing.T) { s, _ := newSMCTestHelper(1) // Notary 0 registers. - err := s.registerNotaries(notaryDeposit, 0, 1) + if err := s.registerNotaries(notaryDeposit, 0, 1); err != nil { + t.Error(err) + } s.fastForward(1) // Proposer adds header consists shard 0, period 1 and chunkroot 0xA. @@ -661,20 +677,17 @@ func TestSubmitVoteTwice(t *testing.T) { shard0 := big.NewInt(0) index0 := big.NewInt(0) s.testAccounts[0].txOpts.Value = big.NewInt(0) - err = s.addHeader(&s.testAccounts[0], shard0, period1, 'A') - if err != nil { + if err := s.addHeader(&s.testAccounts[0], shard0, period1, 'A'); err != nil { t.Errorf("Proposer adds header failed: %v", err) } // Notary 0 votes on header. - err = s.submitVote(&s.testAccounts[0], shard0, period1, index0, 'A') - if err != nil { + if err := s.submitVote(&s.testAccounts[0], shard0, period1, index0, 'A'); err != nil { t.Errorf("Notary submits vote failed: %v", err) } // Notary 0 votes on header again, it should fail. - err = s.submitVote(&s.testAccounts[0], shard0, period1, index0, 'A') - if err == nil { + if err := s.submitVote(&s.testAccounts[0], shard0, period1, index0, 'A'); err == nil { t.Errorf("notary voting twice should have failed") } @@ -716,7 +729,9 @@ func TestSubmitVoteByNonEligibleNotary(t *testing.T) { func TestSubmitVoteWithOutAHeader(t *testing.T) { s, _ := newSMCTestHelper(1) // Notary 0 registers. - err := s.registerNotaries(notaryDeposit, 0, 1) + if err := s.registerNotaries(notaryDeposit, 0, 1); err != nil { + t.Error(err) + } s.fastForward(1) // Proposer adds header consists shard 0, period 1 and chunkroot 0xA. @@ -726,8 +741,7 @@ func TestSubmitVoteWithOutAHeader(t *testing.T) { s.testAccounts[0].txOpts.Value = big.NewInt(0) // Notary 0 votes on header, it should fail because no header has added. - err = s.submitVote(&s.testAccounts[0], shard0, period1, index0, 'A') - if err == nil { + if err := s.submitVote(&s.testAccounts[0], shard0, period1, index0, 'A'); err == nil { t.Errorf("Notary votes should have failed due to missing header") } @@ -742,7 +756,9 @@ func TestSubmitVoteWithOutAHeader(t *testing.T) { func TestSubmitVoteWithInvalidArgs(t *testing.T) { s, _ := newSMCTestHelper(1) // Notary 0 registers. - err := s.registerNotaries(notaryDeposit, 0, 1) + if err := s.registerNotaries(notaryDeposit, 0, 1); err != nil { + t.Error(err) + } s.fastForward(1) // Proposer adds header consists shard 0, period 1 and chunkroot 0xA. @@ -750,21 +766,18 @@ func TestSubmitVoteWithInvalidArgs(t *testing.T) { shard0 := big.NewInt(0) index0 := big.NewInt(0) s.testAccounts[0].txOpts.Value = big.NewInt(0) - err = s.addHeader(&s.testAccounts[0], shard0, period1, 'A') - if err != nil { + if err := s.addHeader(&s.testAccounts[0], shard0, period1, 'A'); err != nil { t.Errorf("Proposer adds header failed: %v", err) } // Notary voting with incorrect period. period2 := big.NewInt(2) - err = s.submitVote(&s.testAccounts[0], shard0, period2, index0, 'A') - if err == nil { + if err := s.submitVote(&s.testAccounts[0], shard0, period2, index0, 'A'); err == nil { t.Errorf("Notary votes should have failed due to incorrect period") } // Notary voting with incorrect chunk root. - err = s.submitVote(&s.testAccounts[0], shard0, period2, index0, 'B') - if err == nil { + if err := s.submitVote(&s.testAccounts[0], shard0, period2, index0, 'B'); err == nil { t.Errorf("Notary votes should have failed due to incorrect chunk root") } } diff --git a/sharding/database/BUILD.bazel b/sharding/database/BUILD.bazel index 577ca9c0c9..dd9f811d8d 100644 --- a/sharding/database/BUILD.bazel +++ b/sharding/database/BUILD.bazel @@ -26,5 +26,6 @@ go_test( "//sharding/types:go_default_library", "@com_github_ethereum_go_ethereum//ethdb:go_default_library", "@com_github_sirupsen_logrus//hooks/test:go_default_library", + "@com_github_syndtr_goleveldb//leveldb/errors:go_default_library", ], ) diff --git a/sharding/database/database_test.go b/sharding/database/database_test.go index eccd7b4f06..49ad46996a 100644 --- a/sharding/database/database_test.go +++ b/sharding/database/database_test.go @@ -6,6 +6,7 @@ import ( "github.com/prysmaticlabs/geth-sharding/sharding/types" logTest "github.com/sirupsen/logrus/hooks/test" + leveldberrors "github.com/syndtr/goleveldb/leveldb/errors" ) // Verifies that ShardDB implements the sharding Service inteface. @@ -106,6 +107,9 @@ func Test_DBGet(t *testing.T) { key2 := []byte{} val2, err := testDB.db.Get(key2) + if err == nil || err.Error() != leveldberrors.ErrNotFound.Error() { + t.Errorf("Expected error %v but got %v", leveldberrors.ErrNotFound, err) + } if len(val2) != 0 { t.Errorf("non-existent key should not have a value. key=%v, value=%v", key2, val2) } diff --git a/sharding/database/inmemory_test.go b/sharding/database/inmemory_test.go index 4ee2282525..c0b4b96b57 100644 --- a/sharding/database/inmemory_test.go +++ b/sharding/database/inmemory_test.go @@ -61,6 +61,9 @@ func Test_ShardKVGet(t *testing.T) { key2 := []byte{} val2, err := kv.Get(key2) + if err == nil { + t.Error("kv.Get for non-existent key should have returned an error") + } if len(val2) != 0 { t.Errorf("non-existent key should not have a value. key=%v, value=%v", key2, val2) } diff --git a/sharding/main.go b/sharding/main.go index 24a8cad861..54cbed89c6 100644 --- a/sharding/main.go +++ b/sharding/main.go @@ -50,10 +50,7 @@ VERSION: app.Before = func(ctx *cli.Context) error { runtime.GOMAXPROCS(runtime.NumCPU()) - if err := debug.Setup(ctx); err != nil { - return err - } - return nil + return debug.Setup(ctx) } app.After = func(ctx *cli.Context) error { diff --git a/sharding/mainchain/BUILD.bazel b/sharding/mainchain/BUILD.bazel index 6d4dbc506e..abf49db75c 100644 --- a/sharding/mainchain/BUILD.bazel +++ b/sharding/mainchain/BUILD.bazel @@ -30,7 +30,6 @@ go_test( srcs = ["smc_client_test.go"], embed = [":go_default_library"], deps = [ - "//sharding/contracts:go_default_library", "//sharding/types:go_default_library", "@com_github_ethereum_go_ethereum//accounts/abi/bind/backends:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", diff --git a/sharding/mainchain/smc_client.go b/sharding/mainchain/smc_client.go index 68f3195637..fa510d3440 100644 --- a/sharding/mainchain/smc_client.go +++ b/sharding/mainchain/smc_client.go @@ -10,7 +10,6 @@ import ( "fmt" "math/big" "os" - "sync" "time" ethereum "github.com/ethereum/go-ethereum" @@ -41,7 +40,6 @@ type SMCClient struct { keystore *keystore.KeyStore // Keystore containing the single signer. smc *contracts.SMC // The deployed sharding management contract. rpcClient *rpc.Client // The RPC client connection to the main geth node. - lock sync.Mutex // Mutex lock for concurrency management. } // NewSMCClient constructs a new instance of an SMCClient. @@ -209,11 +207,6 @@ func (s *SMCClient) DataDirPath() string { return s.dataDirPath } -// Client to interact with a geth node via JSON-RPC. -func (s *SMCClient) ethereumClient() *ethclient.Client { - return s.client -} - // unlockAccount will unlock the specified account using utils.PasswordFileFlag // or empty string if unset. func (s *SMCClient) unlockAccount(account accounts.Account) error { diff --git a/sharding/mainchain/smc_client_test.go b/sharding/mainchain/smc_client_test.go index 659a899e07..cf11124ae1 100644 --- a/sharding/mainchain/smc_client_test.go +++ b/sharding/mainchain/smc_client_test.go @@ -15,7 +15,6 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params" - "github.com/prysmaticlabs/geth-sharding/sharding/contracts" "github.com/prysmaticlabs/geth-sharding/sharding/types" ) @@ -30,9 +29,6 @@ var _ = types.Service(&SMCClient{}) // mockClient is struct to implement the smcClient methods for testing. type mockClient struct { - smc *contracts.SMC - depositFlag bool - t *testing.T backend *backends.SimulatedBackend blockNumber *big.Int } @@ -104,14 +100,15 @@ func TestWaitForTransaction_TransactionNotMined(t *testing.T) { } receipt, err := client.backend.TransactionReceipt(ctx, tx.Hash()) + if err != nil { + t.Error(err) + } if receipt != nil { t.Errorf("transaction mined despite backend not being committed: %v", receipt) } - err = client.WaitForTransaction(ctx, tx.Hash(), timeout) - if err == nil { + if err = client.WaitForTransaction(ctx, tx.Hash(), timeout); err == nil { t.Error("transaction is supposed to timeout and return a error") } - } func TestWaitForTransaction_IsMinedImmediately(t *testing.T) { @@ -145,7 +142,6 @@ func TestWaitForTransaction_IsMinedImmediately(t *testing.T) { }() client.Commit() wg.Wait() - } func TestWaitForTransaction_TimesOut(t *testing.T) { backend := setup() @@ -169,7 +165,6 @@ func TestWaitForTransaction_TimesOut(t *testing.T) { wg.Done() }() wg.Wait() - } func TestWaitForTransaction_IsCancelledWhenParentCtxCancelled(t *testing.T) { backend := setup() @@ -196,5 +191,4 @@ func TestWaitForTransaction_IsCancelledWhenParentCtxCancelled(t *testing.T) { newCtx.Done() wg.Wait() - } diff --git a/sharding/node/BUILD.bazel b/sharding/node/BUILD.bazel index 273f885a02..9b3f8a3c1f 100644 --- a/sharding/node/BUILD.bazel +++ b/sharding/node/BUILD.bazel @@ -16,11 +16,9 @@ go_library( "//sharding/simulator:go_default_library", "//sharding/syncer:go_default_library", "//sharding/txpool:go_default_library", - "//sharding/types:go_default_library", "//sharding/utils:go_default_library", "//shared:go_default_library", "//shared/debug:go_default_library", - "@com_github_ethereum_go_ethereum//event:go_default_library", "@com_github_ethereum_go_ethereum//node:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", "@com_github_urfave_cli//:go_default_library", diff --git a/sharding/node/backend.go b/sharding/node/backend.go index 1a616ff2f5..abd7028b0b 100644 --- a/sharding/node/backend.go +++ b/sharding/node/backend.go @@ -12,7 +12,6 @@ import ( "syscall" "time" - "github.com/ethereum/go-ethereum/event" "github.com/ethereum/go-ethereum/node" "github.com/prysmaticlabs/geth-sharding/sharding/database" "github.com/prysmaticlabs/geth-sharding/sharding/mainchain" @@ -24,7 +23,6 @@ import ( "github.com/prysmaticlabs/geth-sharding/sharding/simulator" "github.com/prysmaticlabs/geth-sharding/sharding/syncer" "github.com/prysmaticlabs/geth-sharding/sharding/txpool" - "github.com/prysmaticlabs/geth-sharding/sharding/types" "github.com/prysmaticlabs/geth-sharding/sharding/utils" "github.com/prysmaticlabs/geth-sharding/shared" "github.com/prysmaticlabs/geth-sharding/shared/debug" @@ -39,9 +37,6 @@ const shardChainDBName = "shardchaindata" // Ethereum network. type ShardEthereum struct { shardConfig *params.Config // Holds necessary information to configure shards. - txPool *txpool.TXPool // Defines the sharding-specific txpool. To be designed. - actor types.Actor // Either notary, proposer, or observer. - eventFeed *event.Feed // Used to enable P2P related interactions via different sharding actors. // Lifecycle and service stores. services *shared.ServiceRegistry diff --git a/sharding/notary/BUILD.bazel b/sharding/notary/BUILD.bazel index 5864a6e264..9158689096 100644 --- a/sharding/notary/BUILD.bazel +++ b/sharding/notary/BUILD.bazel @@ -14,10 +14,8 @@ go_library( "//sharding/mainchain:go_default_library", "//sharding/p2p:go_default_library", "//sharding/params:go_default_library", - "//sharding/types:go_default_library", "@com_github_ethereum_go_ethereum//accounts:go_default_library", "@com_github_ethereum_go_ethereum//accounts/abi/bind:go_default_library", - "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//core/types:go_default_library", "@com_github_ethereum_go_ethereum//params:go_default_library", "@com_github_sirupsen_logrus//:go_default_library", @@ -33,6 +31,5 @@ go_test( "//sharding/params:go_default_library", "//sharding/types:go_default_library", "@com_github_ethereum_go_ethereum//accounts/abi/bind:go_default_library", - "@com_github_ethereum_go_ethereum//crypto:go_default_library", ], ) diff --git a/sharding/notary/notary.go b/sharding/notary/notary.go index 3285290ed2..2e3c605e6d 100644 --- a/sharding/notary/notary.go +++ b/sharding/notary/notary.go @@ -1,22 +1,18 @@ package notary import ( - "bytes" "context" "errors" "fmt" "math/big" - "time" "github.com/ethereum/go-ethereum/accounts" "github.com/ethereum/go-ethereum/accounts/abi/bind" - "github.com/ethereum/go-ethereum/common" gethTypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/params" "github.com/prysmaticlabs/geth-sharding/sharding/contracts" "github.com/prysmaticlabs/geth-sharding/sharding/mainchain" shardparams "github.com/prysmaticlabs/geth-sharding/sharding/params" - "github.com/prysmaticlabs/geth-sharding/sharding/types" log "github.com/sirupsen/logrus" ) @@ -48,7 +44,7 @@ func subscribeBlockHeaders(reader mainchain.Reader, caller mainchain.ContractCal } if v { - if err := checkSMCForNotary(caller, account, head); err != nil { + if err := checkSMCForNotary(caller, account); err != nil { return fmt.Errorf("unable to watch shards. %v", err) } } @@ -59,7 +55,7 @@ func subscribeBlockHeaders(reader mainchain.Reader, caller mainchain.ContractCal // collation for the available shards in the SMC. The function calls // getEligibleNotary from the SMC and notary a collation if // conditions are met. -func checkSMCForNotary(caller mainchain.ContractCaller, account *accounts.Account, head *gethTypes.Header) error { +func checkSMCForNotary(caller mainchain.ContractCaller, account *accounts.Account) error { log.Info("Checking if we are an eligible collation notary for a shard...") shardCount, err := caller.GetShardCount() if err != nil { @@ -112,158 +108,10 @@ func isAccountInNotaryPool(caller mainchain.ContractCaller, account *accounts.Ac return nreg.Deposited, nil } -// hasAccountBeenDeregistered checks if the account has been deregistered from the notary pool. -func hasAccountBeenDeregistered(caller mainchain.ContractCaller, account *accounts.Account) (bool, error) { - nreg, err := getNotaryRegistry(caller, account) - - if err != nil { - return false, err - } - - return nreg.DeregisteredPeriod.Cmp(big.NewInt(0)) == 1, err -} - -// isLockUpOver checks if the lock up period is over -// which will allow the notary to call the releaseNotary function -// in the SMC and get their deposit back. -func isLockUpOver(caller mainchain.ContractCaller, reader mainchain.Reader, account *accounts.Account) (bool, error) { - - //TODO: When chainreader for tests is implemented, get block using the method - //get BlockByNumber instead of passing as an argument to this function. - nreg, err := getNotaryRegistry(caller, account) - if err != nil { - return false, err - } - block, err := reader.BlockByNumber(context.Background(), nil) - if err != nil { - return false, err - } - - return (block.Number().Int64() / shardparams.DefaultConfig.PeriodLength) > nreg.DeregisteredPeriod.Int64()+shardparams.DefaultConfig.NotaryLockupLength, nil - -} - -func transactionWaiting(client mainchain.EthClient, tx *gethTypes.Transaction, duration time.Duration) error { - - err := client.WaitForTransaction(context.Background(), tx.Hash(), duration) - if err != nil { - return err - } - - receipt, err := client.TransactionReceipt(tx.Hash()) - if err != nil { - return err - } - - if receipt.Status == gethTypes.ReceiptStatusFailed { - return errors.New("transaction was not successful, unable to release Notary") - } - return nil - -} - -func settingCanonicalShardChain(shard types.Shard, manager mainchain.ContractManager, period *big.Int, headerHash *common.Hash) error { - - shardID := shard.ShardID() - collationRecords, err := manager.SMCCaller().CollationRecords(&bind.CallOpts{}, shardID, period) - - if err != nil { - return fmt.Errorf("unable to get collation record: %v", err) - } - - // Logs if quorum has been reached and collation is added to the canonical shard chain - if collationRecords.IsElected { - log.Infof("Shard %v in period %v has chosen the collation with its header hash %v to be added to the canonical shard chain", - shardID, period, headerHash) - - // Setting collation header as canonical in the shard chain - header, err := shard.HeaderByHash(headerHash) - if err != nil { - return fmt.Errorf("unable to set Header from hash: %v", err) - } - - err = shard.SetCanonical(header) - if err != nil { - return fmt.Errorf("unable to add collation to canonical shard chain: %v", err) - } - } - - return nil - -} - -func getCurrentNetworkState(manager mainchain.ContractManager, shard types.Shard, reader mainchain.Reader) (int64, *big.Int, *gethTypes.Block, error) { - - shardcount, err := manager.GetShardCount() - if err != nil { - return 0, nil, nil, fmt.Errorf("could not get shard count: %v", err) - } - - shardID := shard.ShardID() - // checks if the shardID is valid - if shardID.Int64() <= int64(0) || shardID.Int64() > shardcount { - return 0, nil, nil, fmt.Errorf("shardId is invalid, it has to be between %d and %d, instead it is %v", 0, shardcount, shardID) - } - - block, err := reader.BlockByNumber(context.Background(), nil) - if err != nil { - return 0, nil, nil, fmt.Errorf("unable to retrieve block: %v", err) - } - - return shardcount, shardID, block, nil - -} - -func checkCollationPeriod(manager mainchain.ContractManager, block *gethTypes.Block, shardID *big.Int) (*big.Int, *big.Int, error) { - - period := big.NewInt(0).Div(block.Number(), big.NewInt(shardparams.DefaultConfig.PeriodLength)) - collPeriod, err := manager.SMCCaller().LastSubmittedCollation(&bind.CallOpts{}, shardID) - - if err != nil { - return nil, nil, fmt.Errorf("unable to get period from last submitted collation: %v", err) - } - - // Checks if the current period is valid in order to vote for the collation on the shard - if period.Int64() != collPeriod.Int64() { - return nil, nil, fmt.Errorf("period in collation is not equal to current period : %d , %d", collPeriod, period) - } - return period, collPeriod, nil - -} - -func hasNotaryVoted(manager mainchain.ContractManager, shardID *big.Int, poolIndex *big.Int) (bool, error) { - hasVoted, err := manager.SMCCaller().HasVoted(&bind.CallOpts{}, shardID, poolIndex) - - if err != nil { - return false, fmt.Errorf("unable to know if notary voted: %v", err) - } - - return hasVoted, nil -} - -func verifyNotary(manager mainchain.ContractManager, client mainchain.EthClient) (*contracts.Registry, error) { - nreg, err := getNotaryRegistry(manager, client.Account()) - - if err != nil { - return nil, err - } - - if !nreg.Deposited { - return nil, fmt.Errorf("notary has not deposited to the SMC") - } - - // Checking if the pool index is valid - if nreg.PoolIndex.Int64() >= shardparams.DefaultConfig.NotaryCommitteeSize { - return nil, fmt.Errorf("invalid pool index %d as it is more than the committee size of %d", nreg.PoolIndex, shardparams.DefaultConfig.NotaryCommitteeSize) - } - - return nreg, nil -} - // joinNotaryPool checks if the deposit flag is true and the account is a // notary in the SMC. If the account is not in the set, it will deposit ETH // into contract. -func joinNotaryPool(manager mainchain.ContractManager, client mainchain.EthClient, config *shardparams.Config) error { +func joinNotaryPool(manager mainchain.ContractManager, client mainchain.EthClient) error { if !client.DepositFlag() { return errors.New("joinNotaryPool called when deposit flag was not set") } @@ -311,185 +159,3 @@ func joinNotaryPool(manager mainchain.ContractManager, client mainchain.EthClien return nil } - -// leaveNotaryPool causes the notary to deregister and leave the notary pool -// by calling the DeregisterNotary function in the SMC. -func leaveNotaryPool(manager mainchain.ContractManager, client mainchain.EthClient) error { - - if inPool, err := isAccountInNotaryPool(manager, client.Account()); !inPool || err != nil { - if err != nil { - return err - } - return errors.New("account has not been able to be deposited in notary pool") - } - - txOps, err := manager.CreateTXOpts(nil) - if err != nil { - return fmt.Errorf("unable to create txOpts: %v", err) - } - - tx, err := manager.SMCTransactor().DeregisterNotary(txOps) - if err != nil { - return fmt.Errorf("unable to deregister notary: %v", err) - } - - err = client.WaitForTransaction(context.Background(), tx.Hash(), 400) - if err != nil { - return err - } - - receipt, err := client.TransactionReceipt(tx.Hash()) - if err != nil { - return err - } - - if receipt.Status == gethTypes.ReceiptStatusFailed { - return errors.New("transaction was not successful, unable to deregister notary") - } - - if dreg, err := hasAccountBeenDeregistered(manager, client.Account()); !dreg || err != nil { - if err != nil { - return err - } - return errors.New("notary unable to be deregistered successfully from pool") - } - - log.Infof("Notary deregistered from the pool with hash: %s", tx.Hash().Hex()) - return nil - -} - -// releaseNotary releases the Notary from the pool by deleting the notary from -// the registry and transferring back the deposit -func releaseNotary(manager mainchain.ContractManager, client mainchain.EthClient, reader mainchain.Reader) error { - - if dreg, err := hasAccountBeenDeregistered(manager, client.Account()); !dreg || err != nil { - if err != nil { - return err - } - return errors.New("notary has not been deregistered from the pool") - } - - if lockup, err := isLockUpOver(manager, reader, client.Account()); !lockup || err != nil { - if err != nil { - return err - } - return errors.New("lockup period is not over") - } - - txOps, err := manager.CreateTXOpts(nil) - if err != nil { - return fmt.Errorf("unable to create txOpts: %v", err) - } - - tx, err := manager.SMCTransactor().ReleaseNotary(txOps) - if err != nil { - return fmt.Errorf("unable to Release Notary: %v", err) - } - - err = transactionWaiting(client, tx, 400) - if err != nil { - return err - } - - nreg, err := getNotaryRegistry(manager, client.Account()) - if err != nil { - return err - } - - if nreg.Deposited || nreg.Balance.Cmp(big.NewInt(0)) != 0 || nreg.DeregisteredPeriod.Cmp(big.NewInt(0)) != 0 || nreg.PoolIndex.Cmp(big.NewInt(0)) != 0 { - return errors.New("notary unable to be released from the pool") - } - - log.Infof("Notary with address: %s released from pool", client.Account().Address.Hex()) - - return nil - -} - -// submitVote votes for a collation on the shard -// by taking in the shard and the hash of the collation header -func submitVote(shard types.Shard, manager mainchain.ContractManager, client mainchain.EthClient, reader mainchain.Reader, headerHash *common.Hash) error { - - _, shardID, block, err := getCurrentNetworkState(manager, shard, reader) - if err != nil { - return err - } - - period, _, err := checkCollationPeriod(manager, block, shardID) - if err != nil { - return err - } - - nreg, err := verifyNotary(manager, client) - - if err != nil { - return err - } - - collationRecords, err := manager.SMCCaller().CollationRecords(&bind.CallOpts{}, shardID, period) - if err != nil { - return fmt.Errorf("unable to get collation record: %v", err) - } - - chunkroot, err := shard.ChunkRootfromHeaderHash(headerHash) - if err != nil { - return fmt.Errorf("unable to get chunk root: %v", err) - } - - // Checking if the chunkroot is valid - if !bytes.Equal(collationRecords.ChunkRoot[:], chunkroot.Bytes()) { - return fmt.Errorf("submmitted collation header has a different chunkroot to the one saved in the SMC") - - } - - hasVoted, err := hasNotaryVoted(manager, shardID, nreg.PoolIndex) - if err != nil { - return err - } - if hasVoted { - return errors.New("notary has already voted") - } - - inCommitee, err := manager.SMCCaller().GetNotaryInCommittee(&bind.CallOpts{}, shardID) - if err != nil { - return fmt.Errorf("unable to know if notary is in committee: %v", err) - } - - if inCommitee != client.Account().Address { - return errors.New("notary is not eligible to vote in this shard at the current period") - } - - txOps, err := manager.CreateTXOpts(nil) - if err != nil { - return fmt.Errorf("unable to create txOpts: %v", err) - } - - tx, err := manager.SMCTransactor().SubmitVote(txOps, shardID, period, nreg.PoolIndex, collationRecords.ChunkRoot) - if err != nil { - - return fmt.Errorf("unable to submit Vote: %v", err) - } - - err = transactionWaiting(client, tx, 400) - - if err != nil { - return err - } - - hasVoted, err = hasNotaryVoted(manager, shardID, nreg.PoolIndex) - - if err != nil { - return fmt.Errorf("unable to know if notary voted: %v", err) - } - - if !hasVoted { - return errors.New("notary has not voted") - } - - log.Infof("Notary has voted for shard: %v in the %v period", shardID, period) - - err = settingCanonicalShardChain(shard, manager, period, headerHash) - - return err -} diff --git a/sharding/notary/service.go b/sharding/notary/service.go index d34d5e79fb..99c6fd54ac 100644 --- a/sharding/notary/service.go +++ b/sharding/notary/service.go @@ -44,7 +44,7 @@ func (n *Notary) notarizeCollations() { // TODO: handle this better through goroutines. Right now, these methods // are blocking. if n.smcClient.DepositFlag() { - if err := joinNotaryPool(n.smcClient, n.smcClient, n.config); err != nil { + if err := joinNotaryPool(n.smcClient, n.smcClient); err != nil { log.Errorf("Could not fetch current block number: %v", err) return } diff --git a/sharding/notary/service_test.go b/sharding/notary/service_test.go index f5a742276a..962c1e4d47 100644 --- a/sharding/notary/service_test.go +++ b/sharding/notary/service_test.go @@ -1,22 +1,15 @@ package notary import ( - "context" "math/big" "testing" "github.com/ethereum/go-ethereum/accounts/abi/bind" - "github.com/ethereum/go-ethereum/crypto" "github.com/prysmaticlabs/geth-sharding/sharding/internal" shardparams "github.com/prysmaticlabs/geth-sharding/sharding/params" "github.com/prysmaticlabs/geth-sharding/sharding/types" ) -var ( - key, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") - addr = crypto.PubkeyToAddress(key.PublicKey) -) - // Verifies that Notary implements the Actor interface. var _ = types.Actor(&Notary{}) @@ -25,60 +18,10 @@ func TestHasAccountBeenDeregistered(t *testing.T) { client := &internal.MockClient{SMC: smc, T: t, Backend: backend, BlockNumber: 1} client.SetDepositFlag(true) - err := joinNotaryPool(client, client, nil) + err := joinNotaryPool(client, client) if err != nil { t.Error(err) } - - err = leaveNotaryPool(client, client) - - if err != nil { - t.Error(err) - } - - dreg, err := hasAccountBeenDeregistered(client, client.Account()) - - if err != nil { - t.Error(err) - } - - if !dreg { - t.Error("account unable to be deregistered from notary pool") - } -} - -func TestIsLockupOver(t *testing.T) { - backend, smc := internal.SetupMockClient(t) - client := &internal.MockClient{SMC: smc, T: t, Backend: backend} - - client.SetDepositFlag(true) - err := joinNotaryPool(client, client, shardparams.DefaultConfig) - if err != nil { - t.Error(err) - } - - err = leaveNotaryPool(client, client) - - if err != nil { - t.Error(err) - } - - client.FastForward(int(shardparams.DefaultConfig.NotaryLockupLength + 100)) - - err = releaseNotary(client, client, client) - if err != nil { - t.Error(err) - } - - lockup, err := isLockUpOver(client, client, client.Account()) - - if err != nil { - t.Error(err) - } - - if !lockup { - t.Error("lockup period is not over despite account being relased from registry") - } } func TestIsAccountInNotaryPool(t *testing.T) { @@ -122,13 +65,13 @@ func TestJoinNotaryPool(t *testing.T) { } client.SetDepositFlag(false) - err = joinNotaryPool(client, client, shardparams.DefaultConfig) + err = joinNotaryPool(client, client) if err == nil { t.Error("joined notary pool while --deposit was not present") } client.SetDepositFlag(true) - err = joinNotaryPool(client, client, shardparams.DefaultConfig) + err = joinNotaryPool(client, client) if err != nil { t.Error(err) } @@ -143,7 +86,7 @@ func TestJoinNotaryPool(t *testing.T) { } // Join while deposited should do nothing. - err = joinNotaryPool(client, client, shardparams.DefaultConfig) + err = joinNotaryPool(client, client) if err != nil { t.Error(err) } @@ -156,107 +99,3 @@ func TestJoinNotaryPool(t *testing.T) { t.Errorf("unexpected number of notaries. Got %d, wanted 1", numNotaries) } } - -func TestLeaveNotaryPool(t *testing.T) { - backend, smc := internal.SetupMockClient(t) - client := &internal.MockClient{SMC: smc, T: t, Backend: backend} - client.SetDepositFlag(true) - - // Test leaving notary pool before joining it. - err := leaveNotaryPool(client, client) - if err == nil { - t.Error("able to leave notary pool despite having not joined it") - } - - // Roundtrip test, join and leave pool. - err = joinNotaryPool(client, client, shardparams.DefaultConfig) - if err != nil { - t.Error(err) - } - client.CommitWithBlock() - - // Now there should be one notary. - numNotaries, err := smc.NotaryPoolLength(&bind.CallOpts{}) - if err != nil { - t.Error(err) - } - if big.NewInt(1).Cmp(numNotaries) != 0 { - t.Errorf("unexpected number of notaries. Got %d, wanted 1", numNotaries) - } - - err = leaveNotaryPool(client, client) - if err != nil { - t.Error(err) - } - client.CommitWithBlock() - - numNotaries, err = smc.NotaryPoolLength(&bind.CallOpts{}) - if err != nil { - t.Error(err) - } - if big.NewInt(0).Cmp(numNotaries) != 0 { - t.Errorf("unexpected number of notaries. Got %d, wanted 0", numNotaries) - } -} - -func TestReleaseNotary(t *testing.T) { - backend, smc := internal.SetupMockClient(t) - client := &internal.MockClient{SMC: smc, T: t, Backend: backend} - client.SetDepositFlag(true) - - // Test release notary before joining it. - err := releaseNotary(client, client, client) - if err == nil { - t.Error("released From notary despite never joining pool") - } - - // Roundtrip test, join and leave pool. - err = joinNotaryPool(client, client, shardparams.DefaultConfig) - if err != nil { - t.Error(err) - } - - err = leaveNotaryPool(client, client) - if err != nil { - t.Error(err) - } - - balance, err := backend.BalanceAt(context.Background(), addr, nil) - if err != nil { - t.Error("unable to retrieve balance") - } - client.FastForward(int(shardparams.DefaultConfig.NotaryLockupLength + 10)) - - err = releaseNotary(client, client, client) - if err != nil { - t.Fatal(err) - } - - nreg, err := smc.NotaryRegistry(&bind.CallOpts{}, addr) - if err != nil { - t.Fatal(err) - } - if nreg.Deposited { - t.Error("Unable to release Notary and deposit money back") - } - - newbalance, err := client.Backend.BalanceAt(context.Background(), addr, nil) - if err != nil { - t.Error("unable to retrieve balance") - } - - if balance.Cmp(newbalance) != -1 { - t.Errorf("Deposit was not returned, balance is currently: %v", newbalance) - } -} - -func TestSubmitVote(t *testing.T) { - backend, smc := internal.SetupMockClient(t) - client := &internal.MockClient{SMC: smc, T: t, Backend: backend} - client.SetDepositFlag(true) - - err := joinNotaryPool(client, client, shardparams.DefaultConfig) - if err != nil { - t.Error(err) - } -} diff --git a/sharding/proposer/BUILD.bazel b/sharding/proposer/BUILD.bazel index 9515a6d3a6..504b1ecbeb 100644 --- a/sharding/proposer/BUILD.bazel +++ b/sharding/proposer/BUILD.bazel @@ -35,6 +35,5 @@ go_test( "@com_github_ethereum_go_ethereum//accounts/abi/bind:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//core/types:go_default_library", - "@com_github_ethereum_go_ethereum//crypto:go_default_library", ], ) diff --git a/sharding/proposer/service_test.go b/sharding/proposer/service_test.go index 2c3b308b94..875ca76112 100644 --- a/sharding/proposer/service_test.go +++ b/sharding/proposer/service_test.go @@ -8,16 +8,10 @@ import ( "github.com/ethereum/go-ethereum/accounts/abi/bind" "github.com/ethereum/go-ethereum/common" gethTypes "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/crypto" "github.com/prysmaticlabs/geth-sharding/sharding/internal" "github.com/prysmaticlabs/geth-sharding/sharding/params" ) -var ( - key, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") - addr = crypto.PubkeyToAddress(key.PublicKey) -) - func TestCreateCollation(t *testing.T) { backend, smc := internal.SetupMockClient(t) node := &internal.MockClient{SMC: smc, T: t, Backend: backend} @@ -29,7 +23,7 @@ func TestCreateCollation(t *testing.T) { nil, 0, nil, data)) } - collation, err := createCollation(node, node.Account(), node, big.NewInt(0), big.NewInt(1), txs) + _, err := createCollation(node, node.Account(), node, big.NewInt(0), big.NewInt(1), txs) if err != nil { t.Fatalf("Create collation failed: %v", err) } @@ -40,7 +34,7 @@ func TestCreateCollation(t *testing.T) { } // negative test case #1: create collation with shard > shardCount. - collation, err = createCollation(node, node.Account(), node, big.NewInt(101), big.NewInt(2), txs) + _, err = createCollation(node, node.Account(), node, big.NewInt(101), big.NewInt(2), txs) if err == nil { t.Errorf("Create collation should have failed with invalid shard number") } @@ -52,13 +46,13 @@ func TestCreateCollation(t *testing.T) { badTxs = append(badTxs, gethTypes.NewTransaction(0, common.HexToAddress("0x0"), nil, 0, nil, data)) } - collation, err = createCollation(node, node.Account(), node, big.NewInt(0), big.NewInt(2), badTxs) + _, err = createCollation(node, node.Account(), node, big.NewInt(0), big.NewInt(2), badTxs) if err == nil { t.Errorf("Create collation should have failed with Txs longer than collation body limit") } // normal test case #1 create collation with correct parameters. - collation, err = createCollation(node, node.Account(), node, big.NewInt(5), big.NewInt(5), txs) + collation, err := createCollation(node, node.Account(), node, big.NewInt(5), big.NewInt(5), txs) if err != nil { t.Errorf("Create collation failed: %v", err) } @@ -117,11 +111,10 @@ func TestAddCollation(t *testing.T) { } // negative test case #1 create the same collation that just got added to SMC. - collation, err = createCollation(node, node.Account(), node, big.NewInt(0), big.NewInt(1), txs) + _, err = createCollation(node, node.Account(), node, big.NewInt(0), big.NewInt(1), txs) if err == nil { t.Errorf("Create collation should fail due to same collation in SMC") } - } func TestCheckCollation(t *testing.T) { @@ -160,6 +153,9 @@ func TestCheckCollation(t *testing.T) { } // normal test case 2: check if we can add header for period 2, should return true. a, err = checkHeaderAdded(node, big.NewInt(0), big.NewInt(2)) + if err != nil { + t.Error(err) + } if !a { t.Errorf("Check header submitted shouldn't return: %v", a) } diff --git a/sharding/simulator/service.go b/sharding/simulator/service.go index 7bc99c93d9..d8c4d5de7c 100644 --- a/sharding/simulator/service.go +++ b/sharding/simulator/service.go @@ -48,8 +48,8 @@ func (s *Simulator) Start() { s.requestFeed = s.p2p.Feed(messages.CollationBodyRequest{}) - go s.broadcastTransactions(time.Tick(s.delay), s.ctx.Done()) - go s.simulateNotaryRequests(s.client.SMCCaller(), s.client.ChainReader(), time.Tick(s.delay), s.ctx.Done()) + go s.broadcastTransactions(time.NewTicker(s.delay).C, s.ctx.Done()) + go s.simulateNotaryRequests(s.client.SMCCaller(), s.client.ChainReader(), time.NewTicker(s.delay).C, s.ctx.Done()) } // Stop the main loop for simulator requests. diff --git a/sharding/simulator/service_test.go b/sharding/simulator/service_test.go index 7ecbcc9f86..ca7af82165 100644 --- a/sharding/simulator/service_test.go +++ b/sharding/simulator/service_test.go @@ -97,7 +97,7 @@ func TestStartStop(t *testing.T) { t.Fatalf("Unable to setup p2p server: %v", err) } - simulator, err := NewSimulator(params.DefaultConfig, &mainchain.SMCClient{}, server, shardID, 0) + simulator, err := NewSimulator(params.DefaultConfig, &mainchain.SMCClient{}, server, shardID, 1*time.Second) if err != nil { t.Fatalf("Unable to setup simulator service: %v", err) } diff --git a/sharding/syncer/handlers_test.go b/sharding/syncer/handlers_test.go index 5cff2de832..f158e652a4 100644 --- a/sharding/syncer/handlers_test.go +++ b/sharding/syncer/handlers_test.go @@ -112,13 +112,8 @@ func (m *mockNode) BlockByNumber(ctx context.Context, number *big.Int) (*types.B type faultyRequest struct{} type faultyCollationFetcher struct{} -type mockSigner struct{} type mockCollationFetcher struct{} -func (m *mockSigner) Sign(hash common.Hash) ([]byte, error) { - return []byte{}, nil -} - func (m *mockCollationFetcher) CollationByHeaderHash(headerHash *common.Hash) (*shardingTypes.Collation, error) { shardID := big.NewInt(1) chunkRoot := common.BytesToHash([]byte{}) @@ -216,6 +211,9 @@ func TestConstructNotaryRequest(t *testing.T) { // Adds the header to the SMC. opt, err := node.CreateTXOpts(big.NewInt(0)) + if err != nil { + t.Error(err) + } smc.AddHeader(opt, shardID, period, chunkRoot, [32]byte{}) backend.Commit() diff --git a/sharding/types/collation_test.go b/sharding/types/collation_test.go index 13d9e4742a..bb45aa88a2 100644 --- a/sharding/types/collation_test.go +++ b/sharding/types/collation_test.go @@ -12,15 +12,6 @@ import ( "github.com/prysmaticlabs/geth-sharding/shared" ) -// fieldAccess is to access unexported fields in structs in another package -func fieldAccess(i interface{}, fields []string) reflect.Value { - val := reflect.ValueOf(i) - for i := 0; i < len(fields); i++ { - val = reflect.Indirect(val).FieldByName(fields[i]) - } - return val - -} func TestCollation_Transactions(t *testing.T) { header := NewCollationHeader(big.NewInt(1), nil, big.NewInt(1), nil, [32]byte{}) body := []byte{} diff --git a/sharding/utils/customflags.go b/sharding/utils/customflags.go index 0fb7ebc36f..9fa5aa8d6e 100644 --- a/sharding/utils/customflags.go +++ b/sharding/utils/customflags.go @@ -31,8 +31,7 @@ func prefixFor(name string) (prefix string) { } else { prefix = "--" } - - return + return prefix } func prefixedNames(fullName string) (prefixed string) { @@ -44,7 +43,7 @@ func prefixedNames(fullName string) (prefixed string) { prefixed += ", " } } - return + return prefixed } // Custom cli.Flag type which expand the received string to an absolute path. diff --git a/shared/service_registry_test.go b/shared/service_registry_test.go index 5c1db5d9ff..61bc1e1969 100644 --- a/shared/service_registry_test.go +++ b/shared/service_registry_test.go @@ -9,7 +9,6 @@ type mockService struct{} type secondMockService struct{} func (m *mockService) Start() { - return } func (m *mockService) Stop() error { @@ -17,7 +16,6 @@ func (m *mockService) Stop() error { } func (s *secondMockService) Start() { - return } func (s *secondMockService) Stop() error {