From 1a51d2d34c6a6375303023305861516cba39708c Mon Sep 17 00:00:00 2001 From: NguyenNguyen Date: Thu, 4 Apr 2019 11:06:50 +0700 Subject: [PATCH 1/3] Fix #482: Ignore order of masternodes list --- consensus/posv/posv.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/consensus/posv/posv.go b/consensus/posv/posv.go index 7508a24fc8..35177163ac 100644 --- a/consensus/posv/posv.go +++ b/consensus/posv/posv.go @@ -25,6 +25,8 @@ import ( "math/big" "math/rand" "path/filepath" + "reflect" + "sort" "strconv" "sync" "time" @@ -424,9 +426,18 @@ func (c *Posv) verifyCascadingFields(chain consensus.ChainReader, header *types. signers = RemovePenaltiesFromBlock(chain, signers, number-uint64(i)*c.config.Epoch) } } - byteMasterNodes := common.ExtractAddressToBytes(signers) extraSuffix := len(header.Extra) - extraSeal - if !bytes.Equal(header.Extra[extraVanity:extraSuffix], byteMasterNodes) { + masternodesFromCheckpointHeader := common.ExtractAddressFromBytes(header.Extra[extraVanity:extraSuffix]) + validSigners := true + sort.Slice(masternodesFromCheckpointHeader, func(i, j int) bool { + return masternodesFromCheckpointHeader[i].String() <= masternodesFromCheckpointHeader[j].String() + }) + sort.Slice(signers, func(i, j int) bool { + return signers[i].String() <= signers[j].String() + }) + validSigners = reflect.DeepEqual(masternodesFromCheckpointHeader, signers) + if !validSigners { + log.Error("Masternodes lists are different in checkpoint header and snapshot", "number", number, "masternodes_from_checkpoint_header", masternodesFromCheckpointHeader, "masternodes_in_snapshot", signers, "penList", penPenalties) return errInvalidCheckpointSigners } if c.HookVerifyMNs != nil { From 152b564ef7afd40b1c82c10497e46591594c2367 Mon Sep 17 00:00:00 2001 From: NguyenNguyen Date: Thu, 4 Apr 2019 14:20:11 +0700 Subject: [PATCH 2/3] Refactoring and adding unit test of compareSignersLists --- consensus/posv/posv.go | 21 +++++++++++++-------- consensus/posv/posv_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/consensus/posv/posv.go b/consensus/posv/posv.go index 35177163ac..24cb26d059 100644 --- a/consensus/posv/posv.go +++ b/consensus/posv/posv.go @@ -428,14 +428,7 @@ func (c *Posv) verifyCascadingFields(chain consensus.ChainReader, header *types. } extraSuffix := len(header.Extra) - extraSeal masternodesFromCheckpointHeader := common.ExtractAddressFromBytes(header.Extra[extraVanity:extraSuffix]) - validSigners := true - sort.Slice(masternodesFromCheckpointHeader, func(i, j int) bool { - return masternodesFromCheckpointHeader[i].String() <= masternodesFromCheckpointHeader[j].String() - }) - sort.Slice(signers, func(i, j int) bool { - return signers[i].String() <= signers[j].String() - }) - validSigners = reflect.DeepEqual(masternodesFromCheckpointHeader, signers) + validSigners := compareSignersLists(masternodesFromCheckpointHeader, signers) if !validSigners { log.Error("Masternodes lists are different in checkpoint header and snapshot", "number", number, "masternodes_from_checkpoint_header", masternodesFromCheckpointHeader, "masternodes_in_snapshot", signers, "penList", penPenalties) return errInvalidCheckpointSigners @@ -451,6 +444,18 @@ func (c *Posv) verifyCascadingFields(chain consensus.ChainReader, header *types. return c.verifySeal(chain, header, parents, fullVerify) } +// compare 2 signers lists +// return true if they are same elements, otherwise return false +func compareSignersLists(list1 []common.Address, list2 []common.Address) bool { + sort.Slice(list1, func(i, j int) bool { + return list1[i].String() <= list1[j].String() + }) + sort.Slice(list2, func(i, j int) bool { + return list2[i].String() <= list2[j].String() + }) + return reflect.DeepEqual(list1, list2) +} + func (c *Posv) GetSnapshot(chain consensus.ChainReader, header *types.Header) (*Snapshot, error) { number := header.Number.Uint64() log.Trace("take snapshot", "number", number, "hash", header.Hash()) diff --git a/consensus/posv/posv_test.go b/consensus/posv/posv_test.go index 1c0a5baeb1..1ccf9f7860 100644 --- a/consensus/posv/posv_test.go +++ b/consensus/posv/posv_test.go @@ -47,3 +47,29 @@ func TestGetM1M2FromCheckpointHeader(t *testing.T) { } } } + +func TestCompareSignersLists(t *testing.T) { + list1 := []common.Address{ + common.StringToAddress("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), + common.StringToAddress("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"), + common.StringToAddress("cccccccccccccccccccccccccccccccccccccccc"), + common.StringToAddress("dddddddddddddddddddddddddddddddddddddddd"), + } + list2 := []common.Address{ + common.StringToAddress("cccccccccccccccccccccccccccccccccccccccc"), + common.StringToAddress("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"), + common.StringToAddress("dddddddddddddddddddddddddddddddddddddddd"), + common.StringToAddress("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"), + } + list3 := []common.Address{ + common.StringToAddress("cccccccccccccccccccccccccccccccccccccccc"), + common.StringToAddress("dddddddddddddddddddddddddddddddddddddddd"), + common.StringToAddress("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"), + } + if !compareSignersLists(list1, list2) { + t.Error("list1 should be equal to list2", "list1", list1, "list2", list2) + } + if compareSignersLists(list1, list3) { + t.Error("list1 and list3 should not be same", "list1", list1, "list3", list3) + } +} From 0afd81cc40acded69a404428d413d7bb45de80df Mon Sep 17 00:00:00 2001 From: NguyenNguyen Date: Thu, 11 Apr 2019 14:10:27 +0700 Subject: [PATCH 3/3] Check empty list --- consensus/posv/posv.go | 3 +++ consensus/posv/posv_test.go | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/consensus/posv/posv.go b/consensus/posv/posv.go index 24cb26d059..6804f348d5 100644 --- a/consensus/posv/posv.go +++ b/consensus/posv/posv.go @@ -447,6 +447,9 @@ func (c *Posv) verifyCascadingFields(chain consensus.ChainReader, header *types. // compare 2 signers lists // return true if they are same elements, otherwise return false func compareSignersLists(list1 []common.Address, list2 []common.Address) bool { + if len(list1) == 0 && len(list2) == 0 { + return true + } sort.Slice(list1, func(i, j int) bool { return list1[i].String() <= list1[j].String() }) diff --git a/consensus/posv/posv_test.go b/consensus/posv/posv_test.go index 1ccf9f7860..56c94e7521 100644 --- a/consensus/posv/posv_test.go +++ b/consensus/posv/posv_test.go @@ -72,4 +72,13 @@ func TestCompareSignersLists(t *testing.T) { if compareSignersLists(list1, list3) { t.Error("list1 and list3 should not be same", "list1", list1, "list3", list3) } + if !compareSignersLists([]common.Address{}, []common.Address{}) { + t.Error("Failed with empty list") + } + if !compareSignersLists([]common.Address{common.StringToAddress("cccccccccccccccccccccccccccccccccccccccc")}, []common.Address{common.StringToAddress("cccccccccccccccccccccccccccccccccccccccc")}) { + t.Error("Failed with list has only one signer") + } + if compareSignersLists([]common.Address{common.StringToAddress("aaaaaaaaaaaaaaaa")}, []common.Address{common.StringToAddress("cccccccccccccccccccccccccccccccccccccccc")}) { + t.Error("Failed with list has only one signer") + } }