From f43fceeb4ded6b8ffb0521d8dbc74bfdf470b75c Mon Sep 17 00:00:00 2001 From: rigel rozanski Date: Mon, 17 Jul 2017 22:50:10 -0400 Subject: [PATCH] fix seqkey uses chain and app --- context.go | 32 ++++++++++++++++++++++++-------- modules/nonce/tx.go | 5 ++--- modules/nonce/tx_test.go | 6 ++---- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/context.go b/context.go index 65e9bf8abd..4da626ed3d 100644 --- a/context.go +++ b/context.go @@ -38,6 +38,7 @@ func (a Actor) Equals(b Actor) bool { bytes.Equal(a.Address, b.Address) } +// Empty checks if the actor is not initialized func (a Actor) Empty() bool { return a.ChainID == "" && a.App == "" && len(a.Address) == 0 } @@ -57,19 +58,34 @@ type Context interface { } //////////////////////////////// Sort Interface -// USAGE sort.Sort(ByAddress()) +// USAGE sort.Sort(ByAll()) func (a Actor) String() string { return fmt.Sprintf("%x", a.Address) } -// ByAddress implements sort.Interface for []Actor based on -// the Address field. -type ByAddress []Actor +// ByAll implements sort.Interface for []Actor. +// It sorts be the ChainID, followed by the App, followed by the Address +type ByAll []Actor // Verify the sort interface at compile time -var _ sort.Interface = ByAddress{} +var _ sort.Interface = ByAll{} -func (a ByAddress) Len() int { return len(a) } -func (a ByAddress) Swap(i, j int) { a[i], a[j] = a[j], a[i] } -func (a ByAddress) Less(i, j int) bool { return bytes.Compare(a[i].Address, a[j].Address) == -1 } +func (a ByAll) Len() int { return len(a) } +func (a ByAll) Swap(i, j int) { a[i], a[j] = a[j], a[i] } +func (a ByAll) Less(i, j int) bool { + + if a[i].ChainID < a[j].ChainID { + return true + } + if a[i].ChainID > a[j].ChainID { + return false + } + if a[i].App < a[j].App { + return true + } + if a[i].App > a[j].App { + return false + } + return bytes.Compare(a[i].Address, a[j].Address) == -1 +} diff --git a/modules/nonce/tx.go b/modules/nonce/tx.go index b5eaddb508..a31536e5e8 100644 --- a/modules/nonce/tx.go +++ b/modules/nonce/tx.go @@ -98,11 +98,10 @@ func (n Tx) getSeqKey() (seqKey []byte) { // First copy the list of signers to sort as sort is done in place signers2sort := make([]basecoin.Actor, len(n.Signers)) copy(signers2sort, n.Signers) - sort.Sort(basecoin.ByAddress(n.Signers)) + sort.Sort(basecoin.ByAll(n.Signers)) for _, signer := range n.Signers { - // rigel: use signer.Bytes()... instead of signer.Address - seqKey = append(seqKey, signer.Address...) + seqKey = append(seqKey, signer.Bytes()...) } //seqKey = merkle.SimpleHashFromBinary(n.Signers) return diff --git a/modules/nonce/tx_test.go b/modules/nonce/tx_test.go index 292c5ed230..218686deec 100644 --- a/modules/nonce/tx_test.go +++ b/modules/nonce/tx_test.go @@ -85,15 +85,13 @@ func TestNonce(t *testing.T) { {true, 2, set321, set321}, // other order is the same {false, 2, set321, set321}, // no repetition - // signers from different chain and apps + // signers with different chain-IDs and apps from actors {false, 3, set123, set123Chain2}, // sign with different chain actors {false, 3, set123, set123App2}, // sign with different app actors {false, 3, set123, set123MixedChains}, // sign with mixed chain actor {false, 3, set123, set123MixedApps}, // sign with mixed app actors - // Rigel: this is the problem I was refering to. - // The sig checks are proper. But the seqkey is not unique - // all of these demand 3, as that what is expected for set123 + // signers from different chain-IDs and apps, working {true, 1, set123Chain2, set123Chain2}, {true, 1, set123App2, set123App2}, {true, 1, set123MixedChains, set123MixedChains},