From 7b8b690df57bb2974e3ec1761fadeb49f5311d99 Mon Sep 17 00:00:00 2001 From: bbuttrick <43212522+bbuttrick@users.noreply.github.com> Date: Sat, 28 Mar 2020 16:55:11 -0400 Subject: [PATCH 01/18] Add a ConcurrentRepolls variable to make the polling throughput configurable with a minimum threshold --- main/params.go | 1 + snow/consensus/avalanche/parameters_test.go | 27 +++++---- snow/consensus/avalanche/topological_test.go | 58 +++++++++++-------- snow/consensus/snowball/consensus_test.go | 4 +- snow/consensus/snowball/parameters.go | 8 ++- snow/consensus/snowball/parameters_test.go | 60 +++++++++++++------- snow/engine/avalanche/config_test.go | 11 ++-- snow/engine/avalanche/transitive_test.go | 11 ++-- snow/engine/avalanche/voter.go | 2 +- snow/engine/snowman/config_test.go | 9 +-- snow/engine/snowman/transitive_test.go | 22 +++---- snow/engine/snowman/voter.go | 2 +- 12 files changed, 128 insertions(+), 87 deletions(-) diff --git a/main/params.go b/main/params.go index b8961c3..e91179c 100644 --- a/main/params.go +++ b/main/params.go @@ -93,6 +93,7 @@ func init() { fs.IntVar(&Config.ConsensusParams.BetaRogue, "snow-rogue-commit-threshold", 30, "Beta value to use for rogue transactions") fs.IntVar(&Config.ConsensusParams.Parents, "snow-avalanche-num-parents", 5, "Number of vertexes for reference from each new vertex") fs.IntVar(&Config.ConsensusParams.BatchSize, "snow-avalanche-batch-size", 30, "Number of operations to batch in each new vertex") + fs.IntVar(&Config.ConsensusParams.ConcurrentRepolls, "snow-concurrent-repolls", 1, "Minimum number of concurrent polls for finalizing consensus") // Enable/Disable APIs: fs.BoolVar(&Config.AdminAPIEnabled, "api-admin-enabled", true, "If true, this node exposes the Admin API") diff --git a/snow/consensus/avalanche/parameters_test.go b/snow/consensus/avalanche/parameters_test.go index aa28f98..8935cf5 100644 --- a/snow/consensus/avalanche/parameters_test.go +++ b/snow/consensus/avalanche/parameters_test.go @@ -12,10 +12,11 @@ import ( func TestParametersValid(t *testing.T) { p := Parameters{ Parameters: snowball.Parameters{ - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 1, + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -29,10 +30,11 @@ func TestParametersValid(t *testing.T) { func TestParametersInvalidParents(t *testing.T) { p := Parameters{ Parameters: snowball.Parameters{ - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 1, + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 1, }, Parents: 1, BatchSize: 1, @@ -46,10 +48,11 @@ func TestParametersInvalidParents(t *testing.T) { func TestParametersInvalidBatchSize(t *testing.T) { p := Parameters{ Parameters: snowball.Parameters{ - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 1, + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 0, diff --git a/snow/consensus/avalanche/topological_test.go b/snow/consensus/avalanche/topological_test.go index f43ee5b..569d0b6 100644 --- a/snow/consensus/avalanche/topological_test.go +++ b/snow/consensus/avalanche/topological_test.go @@ -27,11 +27,12 @@ func TestTopologicalTxIssued(t *testing.T) { TxIssuedTest(t, TopologicalFactory{ func TestAvalancheVoting(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 2, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 2, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -106,11 +107,12 @@ func TestAvalancheVoting(t *testing.T) { func TestAvalancheTransitiveVoting(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 2, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 2, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -199,11 +201,12 @@ func TestAvalancheTransitiveVoting(t *testing.T) { func TestAvalancheSplitVoting(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 2, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 2, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -262,11 +265,12 @@ func TestAvalancheSplitVoting(t *testing.T) { func TestAvalancheTransitiveRejection(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 2, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 2, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -363,11 +367,12 @@ func TestAvalancheTransitiveRejection(t *testing.T) { func TestAvalancheVirtuous(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 2, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 2, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -489,6 +494,7 @@ func TestAvalancheIsVirtuous(t *testing.T) { Alpha: 2, BetaVirtuous: 1, BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -572,6 +578,7 @@ func TestAvalancheQuiesce(t *testing.T) { Alpha: 1, BetaVirtuous: 1, BetaRogue: 1, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, @@ -665,6 +672,7 @@ func TestAvalancheOrphans(t *testing.T) { Alpha: 1, BetaVirtuous: math.MaxInt32, BetaRogue: math.MaxInt32, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, diff --git a/snow/consensus/snowball/consensus_test.go b/snow/consensus/snowball/consensus_test.go index 67fec3d..922f606 100644 --- a/snow/consensus/snowball/consensus_test.go +++ b/snow/consensus/snowball/consensus_test.go @@ -22,7 +22,7 @@ func ParamsTest(t *testing.T, factory Factory) { params := Parameters{ Metrics: prometheus.NewRegistry(), - K: 2, Alpha: 2, BetaVirtuous: 1, BetaRogue: 2, + K: 2, Alpha: 2, BetaVirtuous: 1, BetaRogue: 2, ConcurrentRepolls: 1, } sb.Initialize(params, Red) @@ -34,5 +34,7 @@ func ParamsTest(t *testing.T, factory Factory) { t.Fatalf("Wrong Beta1 parameter") } else if p.BetaRogue != params.BetaRogue { t.Fatalf("Wrong Beta2 parameter") + } else if p.ConcurrentRepolls != params.ConcurrentRepolls { + t.Fatalf("Wrong Repoll parameter") } } diff --git a/snow/consensus/snowball/parameters.go b/snow/consensus/snowball/parameters.go index 5e14afa..11932d9 100644 --- a/snow/consensus/snowball/parameters.go +++ b/snow/consensus/snowball/parameters.go @@ -11,9 +11,9 @@ import ( // Parameters required for snowball consensus type Parameters struct { - Namespace string - Metrics prometheus.Registerer - K, Alpha, BetaVirtuous, BetaRogue int + Namespace string + Metrics prometheus.Registerer + K, Alpha, BetaVirtuous, BetaRogue, ConcurrentRepolls int } // Valid returns nil if the parameters describe a valid initialization. @@ -27,6 +27,8 @@ func (p Parameters) Valid() error { return fmt.Errorf("BetaVirtuous = %d: Fails the condition that: 0 < BetaVirtuous", p.BetaVirtuous) case p.BetaRogue < p.BetaVirtuous: return fmt.Errorf("BetaVirtuous = %d, BetaRogue = %d: Fails the condition that: BetaVirtuous <= BetaRogue", p.BetaVirtuous, p.BetaRogue) + case p.ConcurrentRepolls <= 0 || p.ConcurrentRepolls > p.BetaRogue: + return fmt.Errorf("ConcurrentRepolls = %d, BetaRogue = %d: Fails the condition that: 0 < ConcurrentRepolls <= BetaRogue", p.ConcurrentRepolls, p.BetaRogue) default: return nil } diff --git a/snow/consensus/snowball/parameters_test.go b/snow/consensus/snowball/parameters_test.go index de1b666..6860725 100644 --- a/snow/consensus/snowball/parameters_test.go +++ b/snow/consensus/snowball/parameters_test.go @@ -9,10 +9,11 @@ import ( func TestParametersValid(t *testing.T) { p := Parameters{ - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 1, + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 1, } if err := p.Valid(); err != nil { @@ -22,10 +23,11 @@ func TestParametersValid(t *testing.T) { func TestParametersInvalidK(t *testing.T) { p := Parameters{ - K: 0, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 1, + K: 0, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 1, } if err := p.Valid(); err == nil { @@ -35,10 +37,11 @@ func TestParametersInvalidK(t *testing.T) { func TestParametersInvalidAlpha(t *testing.T) { p := Parameters{ - K: 1, - Alpha: 0, - BetaVirtuous: 1, - BetaRogue: 1, + K: 1, + Alpha: 0, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 1, } if err := p.Valid(); err == nil { @@ -48,10 +51,11 @@ func TestParametersInvalidAlpha(t *testing.T) { func TestParametersInvalidBetaVirtuous(t *testing.T) { p := Parameters{ - K: 1, - Alpha: 1, - BetaVirtuous: 0, - BetaRogue: 1, + K: 1, + Alpha: 1, + BetaVirtuous: 0, + BetaRogue: 1, + ConcurrentRepolls: 1, } if err := p.Valid(); err == nil { @@ -61,13 +65,29 @@ func TestParametersInvalidBetaVirtuous(t *testing.T) { func TestParametersInvalidBetaRogue(t *testing.T) { p := Parameters{ - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 0, + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 0, + ConcurrentRepolls: 1, } if err := p.Valid(); err == nil { t.Fatalf("Should have failed due to invalid beta rogue") } } + +func TestParametersInvalidConcurrentRepolls(t *testing.T) { + p := Parameters{ + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, + ConcurrentRepolls: 2, + } + + if err := p.Valid(); err == nil { + t.Fatalf("Should have failed due to invalid concurrent repolls") + } +} + diff --git a/snow/engine/avalanche/config_test.go b/snow/engine/avalanche/config_test.go index 4906559..b11d186 100644 --- a/snow/engine/avalanche/config_test.go +++ b/snow/engine/avalanche/config_test.go @@ -26,11 +26,12 @@ func DefaultConfig() Config { }, Params: avalanche.Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, diff --git a/snow/engine/avalanche/transitive_test.go b/snow/engine/avalanche/transitive_test.go index 6f5b5ed..752c414 100644 --- a/snow/engine/avalanche/transitive_test.go +++ b/snow/engine/avalanche/transitive_test.go @@ -340,11 +340,12 @@ func TestEngineMultipleQuery(t *testing.T) { config.Params = avalanche.Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 3, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 3, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Parents: 2, BatchSize: 1, diff --git a/snow/engine/avalanche/voter.go b/snow/engine/avalanche/voter.go index 72a1b53..1295efc 100644 --- a/snow/engine/avalanche/voter.go +++ b/snow/engine/avalanche/voter.go @@ -58,7 +58,7 @@ func (v *voter) Update() { v.t.Config.Context.Log.Verbo("Avalanche engine can't quiesce") - if len(v.t.polls.m) == 0 { + if len(v.t.polls.m) < v.t.Config.Params.ConcurrentRepolls { v.t.repoll() } } diff --git a/snow/engine/snowman/config_test.go b/snow/engine/snowman/config_test.go index 1b590b7..6cf7c51 100644 --- a/snow/engine/snowman/config_test.go +++ b/snow/engine/snowman/config_test.go @@ -23,10 +23,11 @@ func DefaultConfig() Config { }, Params: snowball.Parameters{ Metrics: prometheus.NewRegistry(), - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 2, + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, }, Consensus: &snowman.Topological{}, } diff --git a/snow/engine/snowman/transitive_test.go b/snow/engine/snowman/transitive_test.go index 1920d8c..5f93941 100644 --- a/snow/engine/snowman/transitive_test.go +++ b/snow/engine/snowman/transitive_test.go @@ -304,11 +304,12 @@ func TestEngineMultipleQuery(t *testing.T) { config := DefaultConfig() config.Params = snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 3, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 3, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, } vdr0 := validators.GenerateRandomValidator(1) @@ -672,11 +673,12 @@ func TestVoteCanceling(t *testing.T) { config := DefaultConfig() config.Params = snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 3, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 3, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, + ConcurrentRepolls: 1, } vdr0 := validators.GenerateRandomValidator(1) diff --git a/snow/engine/snowman/voter.go b/snow/engine/snowman/voter.go index d9c8a7f..db9686b 100644 --- a/snow/engine/snowman/voter.go +++ b/snow/engine/snowman/voter.go @@ -53,7 +53,7 @@ func (v *voter) Update() { v.t.Config.Context.Log.Verbo("Snowman engine can't quiesce") - if len(v.t.polls.m) == 0 { + if len(v.t.polls.m) < v.t.Config.Params.ConcurrentRepolls { v.t.repoll() } } From 6bde51cb7c25e88a7610d26a83dd83d4ef9464e3 Mon Sep 17 00:00:00 2001 From: bb-2 <43212522+bb-2@users.noreply.github.com> Date: Sat, 28 Mar 2020 17:03:45 -0400 Subject: [PATCH 02/18] fix formatting errors --- snow/consensus/avalanche/topological_test.go | 30 ++++++++++---------- snow/engine/avalanche/voter.go | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/snow/consensus/avalanche/topological_test.go b/snow/consensus/avalanche/topological_test.go index 569d0b6..0ad3b0a 100644 --- a/snow/consensus/avalanche/topological_test.go +++ b/snow/consensus/avalanche/topological_test.go @@ -489,11 +489,11 @@ func TestAvalancheVirtuous(t *testing.T) { func TestAvalancheIsVirtuous(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 2, - Alpha: 2, - BetaVirtuous: 1, - BetaRogue: 2, + Metrics: prometheus.NewRegistry(), + K: 2, + Alpha: 2, + BetaVirtuous: 1, + BetaRogue: 2, ConcurrentRepolls: 1, }, Parents: 2, @@ -573,11 +573,11 @@ func TestAvalancheIsVirtuous(t *testing.T) { func TestAvalancheQuiesce(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 1, - Alpha: 1, - BetaVirtuous: 1, - BetaRogue: 1, + Metrics: prometheus.NewRegistry(), + K: 1, + Alpha: 1, + BetaVirtuous: 1, + BetaRogue: 1, ConcurrentRepolls: 1, }, Parents: 2, @@ -667,11 +667,11 @@ func TestAvalancheQuiesce(t *testing.T) { func TestAvalancheOrphans(t *testing.T) { params := Parameters{ Parameters: snowball.Parameters{ - Metrics: prometheus.NewRegistry(), - K: 1, - Alpha: 1, - BetaVirtuous: math.MaxInt32, - BetaRogue: math.MaxInt32, + Metrics: prometheus.NewRegistry(), + K: 1, + Alpha: 1, + BetaVirtuous: math.MaxInt32, + BetaRogue: math.MaxInt32, ConcurrentRepolls: 1, }, Parents: 2, diff --git a/snow/engine/avalanche/voter.go b/snow/engine/avalanche/voter.go index 1295efc..8054ec5 100644 --- a/snow/engine/avalanche/voter.go +++ b/snow/engine/avalanche/voter.go @@ -58,7 +58,7 @@ func (v *voter) Update() { v.t.Config.Context.Log.Verbo("Avalanche engine can't quiesce") - if len(v.t.polls.m) < v.t.Config.Params.ConcurrentRepolls { + if len(v.t.polls.m) < v.t.Config.Params.ConcurrentRepolls { v.t.repoll() } } From bf1a94611ee5adf5331cdd777ef1cf80ba879222 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 16:39:51 +0100 Subject: [PATCH 03/18] scripts: Use argparse for create.py arguments This gives better feedback on what arguments are required $ python3 scripts/aws/create.py usage: create.py [-h] numBootstraps numNodes create.py: error: the following arguments are required: numBootstraps, numNodes $ python3 scripts/aws/create.py -h usage: create.py [-h] numBootstraps numNodes Start a number of AVA nodes on Amazon EC2 positional arguments: numBootstraps numNodes optional arguments: -h, --help show this help message and exit --- scripts/aws/create.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/scripts/aws/create.py b/scripts/aws/create.py index ab7a6d7..84f3451 100644 --- a/scripts/aws/create.py +++ b/scripts/aws/create.py @@ -1,17 +1,15 @@ -import sys +""" +Start a number of AVA nodes on Amazon EC2 +""" + import boto3 -ec2 = boto3.client("ec2") - -# Should be called with python3 aws_create.py $numBootstraps $numNodes -numBootstraps = int(sys.argv[1]) -numNodes = int(sys.argv[2]) bootstapNode = "Borealis-Bootstrap" fullNode = "Borealis-Node" -def runInstances(num: int, name: str): +def runInstances(ec2, num: int, name: str): if num > 0: ec2.run_instances( ImageId="ami-0badd1c10cb7673e9", @@ -28,8 +26,18 @@ def runInstances(num: int, name: str): def main(): - runInstances(numBootstraps, bootstapNode) - runInstances(numNodes, fullNode) + import argparse + + parser = argparse.ArgumentParser( + description=__doc__, + ) + parser.add_argument('numBootstraps', type=int) + parser.add_argument('numNodes', type=int) + args = parser.parse_args() + + ec2 = boto3.client("ec2") + runInstances(ec2, args.numBootstraps, bootstapNode) + runInstances(ec2, args.numNodes, fullNode) if __name__ == "__main__": From a718b32c6ac77ffa11073530829109d687c99e42 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 16:40:31 +0100 Subject: [PATCH 04/18] scripts: Add a hashbang, so aws create.py can be run directly --- scripts/aws/create.py | 1 + 1 file changed, 1 insertion(+) mode change 100644 => 100755 scripts/aws/create.py diff --git a/scripts/aws/create.py b/scripts/aws/create.py old mode 100644 new mode 100755 index 84f3451..07d75e7 --- a/scripts/aws/create.py +++ b/scripts/aws/create.py @@ -1,3 +1,4 @@ +#!/usr/bin/env python3 """ Start a number of AVA nodes on Amazon EC2 """ From 52e3d427a1e567f14eb9cfad80f5869bd29e6586 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 16:53:55 +0100 Subject: [PATCH 05/18] utils: Add unit tests for clocks --- utils/timer/clock_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 utils/timer/clock_test.go diff --git a/utils/timer/clock_test.go b/utils/timer/clock_test.go new file mode 100644 index 0000000..05b18a7 --- /dev/null +++ b/utils/timer/clock_test.go @@ -0,0 +1,37 @@ +package timer + +import ( + "testing" + "time" +) + +func TestClockSet(t *testing.T) { + clock := Clock{false, time.Unix(1000000, 0)} + clock.Set(time.Unix(0, 0)) + if clock.faked == false { + t.Error("Fake time was set, but .faked flag was not set") + } + if !clock.Time().Equal(time.Unix(0, 0)) { + t.Error("Fake time was set, but not returned") + } +} + +func TestClockSync(t *testing.T) { + clock := Clock{true, time.Unix(0, 0)} + clock.Sync() + if clock.faked == true { + t.Error("Clock was synced, but .faked flag was set") + } + if clock.Time().Equal(time.Unix(0, 0)) { + t.Error("Clock was synced, but returned a fake time") + } +} + +func TestClockUnix(t *testing.T) { + clock := Clock{true, time.Unix(-14159040, 0)} + actual := clock.Unix() + if actual != 0 { + // We are Unix of 1970s, Moon landings are irrelevant + t.Errorf("Expected time prior to Unix epoch to be clamped to 0, got %d", actual) + } +} From aef48061c5fc34b20c76f07b190108a04953f5aa Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 16:57:42 +0100 Subject: [PATCH 06/18] utils: Add tests in safe_math: Max64, Min64, Sub64, Diff64 --- utils/math/safe_math_test.go | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/utils/math/safe_math_test.go b/utils/math/safe_math_test.go index c8428b5..47f65f9 100644 --- a/utils/math/safe_math_test.go +++ b/utils/math/safe_math_test.go @@ -10,6 +10,28 @@ import ( const maxUint64 uint64 = math.MaxUint64 +func TestMax64(t *testing.T) { + actual := Max64(0, maxUint64) + if actual != maxUint64 { + t.Fatalf("Expected %d, got %d", maxUint64, actual) + } + actual = Max64(maxUint64, 0) + if actual != maxUint64 { + t.Fatalf("Expected %d, got %d", maxUint64, actual) + } +} + +func TestMin64(t *testing.T) { + actual := Min64(0, maxUint64) + if actual != 0 { + t.Fatalf("Expected %d, got %d", 0, actual) + } + actual = Min64(maxUint64, 0) + if actual != 0 { + t.Fatalf("Expected %d, got %d", 0, actual) + } +} + func TestAdd64(t *testing.T) { sum, err := Add64(0, maxUint64) if err != nil { @@ -51,6 +73,20 @@ func TestAdd64(t *testing.T) { } } +func TestSub64(t *testing.T) { + actual, err := Sub64(2, 1) + if err != nil { + t.Fatalf("Sub64 failed unexpectedly") + } else if actual != 1 { + t.Fatalf("Expected %d, got %d", 1, actual) + } + + _, err = Sub64(1, 2) + if err == nil { + t.Fatalf("Sub64 did not fail in the manner expected") + } +} + func TestMul64(t *testing.T) { if prod, err := Mul64(maxUint64, 0); err != nil { t.Fatalf("Mul64 failed unexpectedly") @@ -68,3 +104,15 @@ func TestMul64(t *testing.T) { t.Fatalf("Mul64 overflowed") } } + +func TestDiff64(t *testing.T) { + actual := Diff64(0, maxUint64) + if actual != maxUint64 { + t.Fatalf("Expected %d, got %d", maxUint64, actual) + } + + actual = Diff64(maxUint64, 0) + if actual != maxUint64 { + t.Fatalf("Expected %d, got %d", maxUint64, actual) + } +} From cd59668be56443ae727eecc98c4df0ab55b5d69b Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:02:12 +0100 Subject: [PATCH 07/18] utils: Add test for Packer.UnpackByte --- utils/wrappers/packing_test.go | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index a97463f..62d1e60 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -8,7 +8,11 @@ import ( "testing" ) -func TestPackerByte(t *testing.T) { +const ( + ByteSentinal = 0 +) + +func TestPackerPackByte(t *testing.T) { p := Packer{MaxSize: 1} p.PackByte(0x01) @@ -25,6 +29,34 @@ func TestPackerByte(t *testing.T) { if !bytes.Equal(p.Bytes, expected) { t.Fatalf("Packer.PackByte wrote:\n%v\nExpected:\n%v", p.Bytes, expected) } + + p.PackByte(0x02) + if !p.Errored() { + t.Fatal("Packer.PackByte did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackByte(t *testing.T) { + var ( + p = Packer{Bytes: []byte{0x01}, Offset: 0} + actual = p.UnpackByte() + expected byte = 1 + expectedLen = ByteLen + ) + if p.Errored() { + t.Fatalf("Packer.UnpackByte unexpectedly raised %s", p.Err) + } else if actual != expected { + t.Fatalf("Packer.UnpackByte returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackByte left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackByte() + if !p.Errored() { + t.Fatalf("Packer.UnpackByte should have set error, due to attempted out of bounds read") + } else if actual != ByteSentinal { + t.Fatalf("Packer.UnpackByte returned %d, expected sentinal value %d", actual, ByteSentinal) + } } func TestPackerShort(t *testing.T) { From 80e67495925a87538cb3a89ce0129173c851a87f Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:05:03 +0100 Subject: [PATCH 08/18] utils: Add test for Packer.UnpackShort --- utils/wrappers/packing_test.go | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 62d1e60..a6e2b50 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -10,6 +10,7 @@ import ( const ( ByteSentinal = 0 + ShortSentinal = 0 ) func TestPackerPackByte(t *testing.T) { @@ -59,7 +60,7 @@ func TestPackerUnpackByte(t *testing.T) { } } -func TestPackerShort(t *testing.T) { +func TestPackerPackShort(t *testing.T) { p := Packer{MaxSize: 2} p.PackShort(0x0102) @@ -78,6 +79,29 @@ func TestPackerShort(t *testing.T) { } } +func TestPackerUnpackShort(t *testing.T) { + var ( + p = Packer{Bytes: []byte{0x01, 0x02}, Offset: 0} + actual = p.UnpackShort() + expected uint16 = 0x0102 + expectedLen = ShortLen + ) + if p.Errored() { + t.Fatalf("Packer.UnpackShort unexpectedly raised %s", p.Err) + } else if actual != expected { + t.Fatalf("Packer.UnpackShort returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackShort left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackShort() + if !p.Errored() { + t.Fatalf("Packer.UnpackShort should have set error, due to attempted out of bounds read") + } else if actual != ShortSentinal { + t.Fatalf("Packer.UnpackShort returned %d, expected sentinal value %d", actual, ShortSentinal) + } +} + func TestPackerInt(t *testing.T) { p := Packer{MaxSize: 4} From 9a4a44a7648639eee0950b2262a34e96d68677de Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:05:58 +0100 Subject: [PATCH 09/18] utils: Add test for Packer.UnpackInt --- utils/wrappers/packing_test.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index a6e2b50..1719b67 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -11,6 +11,7 @@ import ( const ( ByteSentinal = 0 ShortSentinal = 0 + IntSentinal = 0 ) func TestPackerPackByte(t *testing.T) { @@ -102,7 +103,7 @@ func TestPackerUnpackShort(t *testing.T) { } } -func TestPackerInt(t *testing.T) { +func TestPackerPackInt(t *testing.T) { p := Packer{MaxSize: 4} p.PackInt(0x01020304) @@ -119,6 +120,34 @@ func TestPackerInt(t *testing.T) { if !bytes.Equal(p.Bytes, expected) { t.Fatalf("Packer.PackInt wrote:\n%v\nExpected:\n%v", p.Bytes, expected) } + + p.PackInt(0x05060708) + if !p.Errored() { + t.Fatal("Packer.PackInt did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackInt(t *testing.T) { + var ( + p = Packer{Bytes: []byte{0x01, 0x02, 0x03, 0x04}, Offset: 0} + actual = p.UnpackInt() + expected uint32 = 0x01020304 + expectedLen = IntLen + ) + if p.Errored() { + t.Fatalf("Packer.UnpackInt unexpectedly raised %s", p.Err) + } else if actual != expected { + t.Fatalf("Packer.UnpackInt returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackInt left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackInt() + if !p.Errored() { + t.Fatalf("Packer.UnpackInt should have set error, due to attempted out of bounds read") + } else if actual != IntSentinal { + t.Fatalf("Packer.UnpackInt returned %d, expected sentinal value %d", actual, IntSentinal) + } } func TestPackerLong(t *testing.T) { From 3914613731ce3d1ac941ba872d088e461bcb759a Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:07:53 +0100 Subject: [PATCH 10/18] utils: Add test for Packer.UnpackLong --- utils/wrappers/packing_test.go | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 1719b67..1034ebb 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -12,6 +12,7 @@ const ( ByteSentinal = 0 ShortSentinal = 0 IntSentinal = 0 + LongSentinal = 0 ) func TestPackerPackByte(t *testing.T) { @@ -150,7 +151,7 @@ func TestPackerUnpackInt(t *testing.T) { } } -func TestPackerLong(t *testing.T) { +func TestPackerPackLong(t *testing.T) { p := Packer{MaxSize: 8} p.PackLong(0x0102030405060708) @@ -167,6 +168,34 @@ func TestPackerLong(t *testing.T) { if !bytes.Equal(p.Bytes, expected) { t.Fatalf("Packer.PackLong wrote:\n%v\nExpected:\n%v", p.Bytes, expected) } + + p.PackLong(0x090a0b0c0d0e0f00) + if !p.Errored() { + t.Fatal("Packer.PackLong did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackLong(t *testing.T) { + var ( + p = Packer{Bytes: []byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}, Offset: 0} + actual = p.UnpackLong() + expected uint64 = 0x0102030405060708 + expectedLen = LongLen + ) + if p.Errored() { + t.Fatalf("Packer.UnpackLong unexpectedly raised %s", p.Err) + } else if actual != expected { + t.Fatalf("Packer.UnpackLong returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackLong left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackLong() + if !p.Errored() { + t.Fatalf("Packer.UnpackLong should have set error, due to attempted out of bounds read") + } else if actual != LongSentinal { + t.Fatalf("Packer.UnpackLong returned %d, expected sentinal value %d", actual, LongSentinal) + } } func TestPackerString(t *testing.T) { From 66b031efc67483b0535e2dbfd257e314a2f3457b Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:09:20 +0100 Subject: [PATCH 11/18] utils: Add tests for Packer.PackFixedBytes & UnpackFixedBytes --- utils/wrappers/packing_test.go | 47 ++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 1034ebb..23a3b0f 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -198,6 +198,53 @@ func TestPackerUnpackLong(t *testing.T) { } } +func TestPackerPackFixedBytes(t *testing.T) { + p := Packer{MaxSize: 3} + + p.PackFixedBytes([]byte("Ava")) + + if p.Errored() { + t.Fatal(p.Err) + } + + if size := len(p.Bytes); size != 3 { + t.Fatalf("Packer.PackFixedBytes wrote %d byte(s) but expected %d byte(s)", size, 3) + } + + expected := []byte("Ava") + if !bytes.Equal(p.Bytes, expected) { + t.Fatalf("Packer.PackFixedBytes wrote:\n%v\nExpected:\n%v", p.Bytes, expected) + } + + p.PackFixedBytes([]byte("Ava")) + if !p.Errored() { + t.Fatal("Packer.PackFixedBytes did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackFixedBytes(t *testing.T) { + var ( + p = Packer{Bytes: []byte("Ava")} + actual = p.UnpackFixedBytes(3) + expected = []byte("Ava") + expectedLen = 3 + ) + if p.Errored() { + t.Fatalf("Packer.UnpackFixedBytes unexpectedly raised %s", p.Err) + } else if !bytes.Equal(actual, expected) { + t.Fatalf("Packer.UnpackFixedBytes returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackFixedBytes left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackFixedBytes(3) + if !p.Errored() { + t.Fatalf("Packer.UnpackFixedBytes should have set error, due to attempted out of bounds read") + } else if actual != nil { + t.Fatalf("Packer.UnpackFixedBytes returned %v, expected sentinal value %v", actual, nil) + } +} + func TestPackerString(t *testing.T) { p := Packer{MaxSize: 5} From ae9c1549d04a4f3fda0859d6d20cfce0a0893b1f Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:11:04 +0100 Subject: [PATCH 12/18] utils: Add tests for Packer.PackBytes & UnpackBytes --- utils/wrappers/packing_test.go | 47 ++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 23a3b0f..3aacd14 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -245,6 +245,53 @@ func TestPackerUnpackFixedBytes(t *testing.T) { } } +func TestPackerPackBytes(t *testing.T) { + p := Packer{MaxSize: 7} + + p.PackBytes([]byte("Ava")) + + if p.Errored() { + t.Fatal(p.Err) + } + + if size := len(p.Bytes); size != 7 { + t.Fatalf("Packer.PackBytes wrote %d byte(s) but expected %d byte(s)", size, 7) + } + + expected := []byte("\x00\x00\x00\x03Ava") + if !bytes.Equal(p.Bytes, expected) { + t.Fatalf("Packer.PackBytes wrote:\n%v\nExpected:\n%v", p.Bytes, expected) + } + + p.PackBytes([]byte("Ava")) + if !p.Errored() { + t.Fatal("Packer.PackBytes did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackBytes(t *testing.T) { + var ( + p = Packer{Bytes: []byte("\x00\x00\x00\x03Ava")} + actual = p.UnpackBytes() + expected = []byte("Ava") + expectedLen = 7 + ) + if p.Errored() { + t.Fatalf("Packer.UnpackBytes unexpectedly raised %s", p.Err) + } else if !bytes.Equal(actual, expected) { + t.Fatalf("Packer.UnpackBytes returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackBytes left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackBytes() + if !p.Errored() { + t.Fatalf("Packer.UnpackBytes should have set error, due to attempted out of bounds read") + } else if actual != nil { + t.Fatalf("Packer.UnpackBytes returned %v, expected sentinal value %v", actual, nil) + } +} + func TestPackerString(t *testing.T) { p := Packer{MaxSize: 5} From 55cb9d174f2b5a50dd202d6a95cf0ba2ea443bb3 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:40:47 +0100 Subject: [PATCH 13/18] utils: Add tests for Packer.PackFixedByteSlices & UnpackFixedByteSlices --- utils/wrappers/packing.go | 4 ++- utils/wrappers/packing_test.go | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/utils/wrappers/packing.go b/utils/wrappers/packing.go index da9bfc7..d4c0045 100644 --- a/utils/wrappers/packing.go +++ b/utils/wrappers/packing.go @@ -242,7 +242,9 @@ func (p *Packer) PackFixedByteSlices(byteSlices [][]byte) { } } -// UnpackFixedByteSlices unpack a byte slice slice to the byte array +// UnpackFixedByteSlices returns a byte slice slice from the byte array. +// Each byte slice has the specified size. The number of byte slices is +// read from the byte array. func (p *Packer) UnpackFixedByteSlices(size int) [][]byte { sliceSize := p.UnpackInt() bytes := [][]byte(nil) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 3aacd14..8194f55 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -5,6 +5,7 @@ package wrappers import ( "bytes" + "reflect" "testing" ) @@ -292,6 +293,53 @@ func TestPackerUnpackBytes(t *testing.T) { } } +func TestPackerPackFixedByteSlices(t *testing.T) { + p := Packer{MaxSize: 10} + + p.PackFixedByteSlices([][]byte{[]byte("Ava"), []byte("Eva")}) + + if p.Errored() { + t.Fatal(p.Err) + } + + if size := len(p.Bytes); size != 10 { + t.Fatalf("Packer.PackFixedByteSlices wrote %d byte(s) but expected %d byte(s)", size, 13) + } + + expected := []byte("\x00\x00\x00\x02AvaEva") + if !bytes.Equal(p.Bytes, expected) { + t.Fatalf("Packer.PackPackFixedByteSlicesBytes wrote:\n%v\nExpected:\n%v", p.Bytes, expected) + } + + p.PackFixedByteSlices([][]byte{[]byte("Ava"), []byte("Eva")}) + if !p.Errored() { + t.Fatal("Packer.PackFixedByteSlices did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackFixedByteSlices(t *testing.T) { + var ( + p = Packer{Bytes: []byte("\x00\x00\x00\x02AvaEva")} + actual = p.UnpackFixedByteSlices(3) + expected = [][]byte{[]byte("Ava"), []byte("Eva")} + expectedLen = 10 + ) + if p.Errored() { + t.Fatalf("Packer.UnpackFixedByteSlices unexpectedly raised %s", p.Err) + } else if !reflect.DeepEqual(actual, expected) { + t.Fatalf("Packer.UnpackFixedByteSlices returned %d, but expected %d", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackFixedByteSlices left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackFixedByteSlices(3) + if !p.Errored() { + t.Fatalf("Packer.UnpackFixedByteSlices should have set error, due to attempted out of bounds read") + } else if actual != nil { + t.Fatalf("Packer.UnpackFixedByteSlices returned %v, expected sentinal value %v", actual, nil) + } +} + func TestPackerString(t *testing.T) { p := Packer{MaxSize: 5} From 96109a60eaf8da778cc0051ca3fa4d1df39c9d6f Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:42:12 +0100 Subject: [PATCH 14/18] utils: Add tests for Packer.PackBool & UnpackBool --- utils/wrappers/packing.go | 2 ++ utils/wrappers/packing_test.go | 57 ++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/utils/wrappers/packing.go b/utils/wrappers/packing.go index d4c0045..cd00f98 100644 --- a/utils/wrappers/packing.go +++ b/utils/wrappers/packing.go @@ -24,6 +24,8 @@ const ( IntLen = 4 // LongLen is the number of bytes per long LongLen = 8 + // BoolLen is the number of bytes per bool + BoolLen = 1 ) var ( diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 8194f55..9671b76 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -14,6 +14,7 @@ const ( ShortSentinal = 0 IntSentinal = 0 LongSentinal = 0 + BoolSentinal = false ) func TestPackerPackByte(t *testing.T) { @@ -407,3 +408,59 @@ func TestPackBool(t *testing.T) { t.Fatal("got back wrong values") } } + +func TestPackerPackBool(t *testing.T) { + p := Packer{MaxSize: 1} + + p.PackBool(true) + + if p.Errored() { + t.Fatal(p.Err) + } + + if size := len(p.Bytes); size != 1 { + t.Fatalf("Packer.PackBool wrote %d byte(s) but expected %d byte(s)", size, 1) + } + + expected := []byte{0x01} + if !bytes.Equal(p.Bytes, expected) { + t.Fatalf("Packer.PackBool wrote:\n%v\nExpected:\n%v", p.Bytes, expected) + } + + p.PackBool(false) + if !p.Errored() { + t.Fatal("Packer.PackLong did not fail when attempt was beyond p.MaxSize") + } +} + +func TestPackerUnpackBool(t *testing.T) { + var ( + p = Packer{Bytes: []byte{0x01}, Offset: 0} + actual = p.UnpackBool() + expected bool = true + expectedLen = BoolLen + ) + if p.Errored() { + t.Fatalf("Packer.UnpackBool unexpectedly raised %s", p.Err) + } else if actual != expected { + t.Fatalf("Packer.UnpackBool returned %t, but expected %t", actual, expected) + } else if p.Offset != expectedLen { + t.Fatalf("Packer.UnpackBool left Offset %d, expected %d", p.Offset, expectedLen) + } + + actual = p.UnpackBool() + if !p.Errored() { + t.Fatalf("Packer.UnpackBool should have set error, due to attempted out of bounds read") + } else if actual != BoolSentinal { + t.Fatalf("Packer.UnpackBool returned %t, expected sentinal value %t", actual, BoolSentinal) + } + + p = Packer{Bytes: []byte{0x42}, Offset: 0} + expected = false + actual = p.UnpackBool() + if !p.Errored() { + t.Fatalf("Packer.UnpackBool id not raise error for invalid boolean value %v", p.Bytes) + } else if actual != expected { + t.Fatalf("Packer.UnpackBool returned %t, expected sentinal value %t", actual, BoolSentinal) + } +} From 18b76622665f9e8c81f7ca3002dca0b45ebb1d00 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:42:47 +0100 Subject: [PATCH 15/18] utils: Add test for Packer.CheckSpace --- utils/wrappers/packing_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 9671b76..8678f46 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -17,6 +17,32 @@ const ( BoolSentinal = false ) +func TestPackerCheckSpace(t *testing.T) { + p := Packer{Offset: -1} + p.CheckSpace(1) + if !p.Errored() { + t.Fatal("Expected errNegativeOffset") + } + + p = Packer{} + p.CheckSpace(-1) + if !p.Errored() { + t.Fatal("Expected errInvalidInput") + } + + p = Packer{Bytes: []byte{0x01}, Offset: 1} + p.CheckSpace(1) + if !p.Errored() { + t.Fatal("Expected errBadLength") + } + + p = Packer{Bytes: []byte{0x01}, Offset: 2} + p.CheckSpace(0) + if !p.Errored() { + t.Fatal("Expected errBadLength, due to out of bounds offset") + } +} + func TestPackerPackByte(t *testing.T) { p := Packer{MaxSize: 1} From 12117b17289391a95531fc53a4b735993a8bdb9c Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 17:43:18 +0100 Subject: [PATCH 16/18] utils: Add test for Packer.Expand --- utils/wrappers/packing_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/utils/wrappers/packing_test.go b/utils/wrappers/packing_test.go index 8678f46..6937d27 100644 --- a/utils/wrappers/packing_test.go +++ b/utils/wrappers/packing_test.go @@ -43,6 +43,22 @@ func TestPackerCheckSpace(t *testing.T) { } } +func TestPackerExpand(t *testing.T) { + p := Packer{Bytes: []byte{0x01}, Offset: 2} + p.Expand(1) + if !p.Errored() { + t.Fatal("packer.Expand didn't notice packer had out of bounds offset") + } + + p = Packer{Bytes: []byte{0x01, 0x02, 0x03}, Offset: 0} + p.Expand(1) + if p.Errored() { + t.Fatalf("packer.Expand unexpectedly had error %s", p.Err) + } else if len(p.Bytes) != 3 { + t.Fatalf("packer.Expand modified byte array, when it didn't need to") + } +} + func TestPackerPackByte(t *testing.T) { p := Packer{MaxSize: 1} From 8f54a7c1d86481908a4d83b9c93fe562b363b603 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Sun, 29 Mar 2020 19:20:28 +0100 Subject: [PATCH 17/18] utils: Initialise Clock with go defaults in TestClockSet Changed to be more idiomatic, addresses PR feedback. --- utils/timer/clock_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/timer/clock_test.go b/utils/timer/clock_test.go index 05b18a7..ef8eb67 100644 --- a/utils/timer/clock_test.go +++ b/utils/timer/clock_test.go @@ -6,12 +6,12 @@ import ( ) func TestClockSet(t *testing.T) { - clock := Clock{false, time.Unix(1000000, 0)} - clock.Set(time.Unix(0, 0)) + clock := Clock{} + clock.Set(time.Unix(1000000, 0)) if clock.faked == false { t.Error("Fake time was set, but .faked flag was not set") } - if !clock.Time().Equal(time.Unix(0, 0)) { + if !clock.Time().Equal(time.Unix(1000000, 0)) { t.Error("Fake time was set, but not returned") } } From c49b0c0ff66dccde1518ec99d654c049cc6799cb Mon Sep 17 00:00:00 2001 From: bb-2 <43212522+bb-2@users.noreply.github.com> Date: Sun, 29 Mar 2020 21:00:45 -0400 Subject: [PATCH 18/18] Issue multiple polls at vertex/block issuance if below ConcurrentRepoll threshold --- snow/consensus/snowball/parameters.go | 6 ++++-- snow/engine/avalanche/issuer.go | 6 ++++++ snow/engine/snowman/transitive.go | 12 +++++++++--- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/snow/consensus/snowball/parameters.go b/snow/consensus/snowball/parameters.go index 11932d9..6dce13b 100644 --- a/snow/consensus/snowball/parameters.go +++ b/snow/consensus/snowball/parameters.go @@ -27,8 +27,10 @@ func (p Parameters) Valid() error { return fmt.Errorf("BetaVirtuous = %d: Fails the condition that: 0 < BetaVirtuous", p.BetaVirtuous) case p.BetaRogue < p.BetaVirtuous: return fmt.Errorf("BetaVirtuous = %d, BetaRogue = %d: Fails the condition that: BetaVirtuous <= BetaRogue", p.BetaVirtuous, p.BetaRogue) - case p.ConcurrentRepolls <= 0 || p.ConcurrentRepolls > p.BetaRogue: - return fmt.Errorf("ConcurrentRepolls = %d, BetaRogue = %d: Fails the condition that: 0 < ConcurrentRepolls <= BetaRogue", p.ConcurrentRepolls, p.BetaRogue) + case p.ConcurrentRepolls <= 0: + return fmt.Errorf("ConcurrentRepolls = %d: Fails the condition that: 0 < ConcurrentRepolls", p.ConcurrentRepolls) + case p.ConcurrentRepolls > p.BetaRogue: + return fmt.Errorf("ConcurrentRepolls = %d, BetaRogue = %d: Fails the condition that: ConcurrentRepolls <= BetaRogue", p.ConcurrentRepolls, p.BetaRogue) default: return nil } diff --git a/snow/engine/avalanche/issuer.go b/snow/engine/avalanche/issuer.go index befe973..427fe3c 100644 --- a/snow/engine/avalanche/issuer.go +++ b/snow/engine/avalanche/issuer.go @@ -65,8 +65,10 @@ func (i *issuer) Update() { } i.t.RequestID++ + polled := false if numVdrs := len(vdrs); numVdrs == p.K && i.t.polls.Add(i.t.RequestID, vdrSet.Len()) { i.t.Config.Sender.PushQuery(vdrSet, i.t.RequestID, vtxID, i.vtx.Bytes()) + polled = true } else if numVdrs < p.K { i.t.Config.Context.Log.Error("Query for %s was dropped due to an insufficient number of validators", vtxID) } @@ -75,6 +77,10 @@ func (i *issuer) Update() { for _, tx := range i.vtx.Txs() { i.t.txBlocked.Fulfill(tx.ID()) } + + if polled && len(i.t.polls.m) < i.t.Params.ConcurrentRepolls { + i.t.repoll() + } } type vtxIssuer struct{ i *issuer } diff --git a/snow/engine/snowman/transitive.go b/snow/engine/snowman/transitive.go index e023a7d..947967a 100644 --- a/snow/engine/snowman/transitive.go +++ b/snow/engine/snowman/transitive.go @@ -305,7 +305,7 @@ func (t *Transitive) pullSample(blkID ids.ID) { } } -func (t *Transitive) pushSample(blk snowman.Block) { +func (t *Transitive) pushSample(blk snowman.Block) bool { t.Config.Context.Log.Verbo("About to sample from: %s", t.Config.Validators) p := t.Consensus.Parameters() vdrs := t.Config.Validators.Sample(p.K) @@ -315,11 +315,14 @@ func (t *Transitive) pushSample(blk snowman.Block) { } t.RequestID++ + queryIssued := false if numVdrs := len(vdrs); numVdrs == p.K && t.polls.Add(t.RequestID, vdrSet.Len()) { t.Config.Sender.PushQuery(vdrSet, t.RequestID, blk.ID(), blk.Bytes()) + queryIssued = true } else if numVdrs < p.K { t.Config.Context.Log.Error("Query for %s was dropped due to an insufficient number of validators", blk.ID()) } + return queryIssued } func (t *Transitive) deliver(blk snowman.Block) { @@ -338,9 +341,8 @@ func (t *Transitive) deliver(blk snowman.Block) { } t.Config.Context.Log.Verbo("Adding block to consensus: %s", blkID) - t.Consensus.Add(blk) - t.pushSample(blk) + polled := t.pushSample(blk) added := []snowman.Block{} dropped := []snowman.Block{} @@ -373,6 +375,10 @@ func (t *Transitive) deliver(blk snowman.Block) { t.blocked.Abandon(blkID) } + if polled && len(t.polls.m) < t.Params.ConcurrentRepolls { + t.repoll() + } + // Tracks performance statistics t.numBlkRequests.Set(float64(t.blkReqs.Len())) t.numBlockedBlk.Set(float64(t.pending.Len()))