From c9e19ffd0a0d094b2f605a580608ebfa07a02211 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 25 Aug 2021 17:15:48 -0400 Subject: [PATCH 01/11] Register keys with ParticipationRegistry OnNewBlock. --- data/account/msgp_gen.go | 148 ++++++++++++++------- data/account/msgp_gen_test.go | 3 - data/account/participation.go | 33 +++-- data/account/participationRegistry_test.go | 119 ++++------------- node/node.go | 14 +- 5 files changed, 158 insertions(+), 159 deletions(-) diff --git a/data/account/msgp_gen.go b/data/account/msgp_gen.go index 3c2370f167..6df260ec8f 100644 --- a/data/account/msgp_gen.go +++ b/data/account/msgp_gen.go @@ -20,55 +20,73 @@ import ( func (z *participationIDData) MarshalMsg(b []byte) (o []byte) { o = msgp.Require(b, z.Msgsize()) // omitempty: check for empty values - zb0001Len := uint32(5) - var zb0001Mask uint8 /* 6 bits */ + zb0001Len := uint32(7) + var zb0001Mask uint16 /* 9 bits */ if (*z).Parent.MsgIsZero() { - zb0001Len-- - zb0001Mask |= 0x2 - } - if (*z).FirstValid.MsgIsZero() { zb0001Len-- zb0001Mask |= 0x4 } - if (*z).KeyDilution == 0 { + if (*z).KeyregTxnFields.Nonparticipation == false { zb0001Len-- zb0001Mask |= 0x8 } - if (*z).LastValid.MsgIsZero() { + if (*z).KeyregTxnFields.SelectionPK.MsgIsZero() { zb0001Len-- zb0001Mask |= 0x10 } - if (*z).VRFSK.MsgIsZero() { + if (*z).KeyregTxnFields.VoteFirst.MsgIsZero() { zb0001Len-- zb0001Mask |= 0x20 } + if (*z).KeyregTxnFields.VoteKeyDilution == 0 { + zb0001Len-- + zb0001Mask |= 0x40 + } + if (*z).KeyregTxnFields.VotePK.MsgIsZero() { + zb0001Len-- + zb0001Mask |= 0x80 + } + if (*z).KeyregTxnFields.VoteLast.MsgIsZero() { + zb0001Len-- + zb0001Mask |= 0x100 + } // variable map header, size zb0001Len o = append(o, 0x80|uint8(zb0001Len)) if zb0001Len != 0 { - if (zb0001Mask & 0x2) == 0 { // if not empty + if (zb0001Mask & 0x4) == 0 { // if not empty // string "addr" o = append(o, 0xa4, 0x61, 0x64, 0x64, 0x72) o = (*z).Parent.MarshalMsg(o) } - if (zb0001Mask & 0x4) == 0 { // if not empty - // string "fv" - o = append(o, 0xa2, 0x66, 0x76) - o = (*z).FirstValid.MarshalMsg(o) - } if (zb0001Mask & 0x8) == 0 { // if not empty - // string "kd" - o = append(o, 0xa2, 0x6b, 0x64) - o = msgp.AppendUint64(o, (*z).KeyDilution) + // string "nonpart" + o = append(o, 0xa7, 0x6e, 0x6f, 0x6e, 0x70, 0x61, 0x72, 0x74) + o = msgp.AppendBool(o, (*z).KeyregTxnFields.Nonparticipation) } if (zb0001Mask & 0x10) == 0 { // if not empty - // string "lv" - o = append(o, 0xa2, 0x6c, 0x76) - o = (*z).LastValid.MarshalMsg(o) + // string "selkey" + o = append(o, 0xa6, 0x73, 0x65, 0x6c, 0x6b, 0x65, 0x79) + o = (*z).KeyregTxnFields.SelectionPK.MarshalMsg(o) } if (zb0001Mask & 0x20) == 0 { // if not empty - // string "vrfsk" - o = append(o, 0xa5, 0x76, 0x72, 0x66, 0x73, 0x6b) - o = (*z).VRFSK.MarshalMsg(o) + // string "votefst" + o = append(o, 0xa7, 0x76, 0x6f, 0x74, 0x65, 0x66, 0x73, 0x74) + o = (*z).KeyregTxnFields.VoteFirst.MarshalMsg(o) + } + if (zb0001Mask & 0x40) == 0 { // if not empty + // string "votekd" + o = append(o, 0xa6, 0x76, 0x6f, 0x74, 0x65, 0x6b, 0x64) + o = msgp.AppendUint64(o, (*z).KeyregTxnFields.VoteKeyDilution) + } + if (zb0001Mask & 0x80) == 0 { // if not empty + // string "votekey" + o = append(o, 0xa7, 0x76, 0x6f, 0x74, 0x65, 0x6b, 0x65, 0x79) + o = (*z).KeyregTxnFields.VotePK.MarshalMsg(o) + } + if (zb0001Mask & 0x100) == 0 { // if not empty + // string "votelst" + o = append(o, 0xa7, 0x76, 0x6f, 0x74, 0x65, 0x6c, 0x73, 0x74) + o = (*z).KeyregTxnFields.VoteLast.MarshalMsg(o) } } return @@ -94,41 +112,57 @@ func (z *participationIDData) UnmarshalMsg(bts []byte) (o []byte, err error) { } if zb0001 > 0 { zb0001-- - bts, err = (*z).Parent.UnmarshalMsg(bts) + bts, err = (*z).KeyregTxnFields.VotePK.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "Parent") + err = msgp.WrapError(err, "struct-from-array", "VotePK") + return + } + } + if zb0001 > 0 { + zb0001-- + bts, err = (*z).KeyregTxnFields.SelectionPK.UnmarshalMsg(bts) + if err != nil { + err = msgp.WrapError(err, "struct-from-array", "SelectionPK") return } } if zb0001 > 0 { zb0001-- - bts, err = (*z).VRFSK.UnmarshalMsg(bts) + bts, err = (*z).KeyregTxnFields.VoteFirst.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "VRFSK") + err = msgp.WrapError(err, "struct-from-array", "VoteFirst") return } } if zb0001 > 0 { zb0001-- - bts, err = (*z).FirstValid.UnmarshalMsg(bts) + bts, err = (*z).KeyregTxnFields.VoteLast.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "FirstValid") + err = msgp.WrapError(err, "struct-from-array", "VoteLast") return } } if zb0001 > 0 { zb0001-- - bts, err = (*z).LastValid.UnmarshalMsg(bts) + (*z).KeyregTxnFields.VoteKeyDilution, bts, err = msgp.ReadUint64Bytes(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "LastValid") + err = msgp.WrapError(err, "struct-from-array", "VoteKeyDilution") return } } if zb0001 > 0 { zb0001-- - (*z).KeyDilution, bts, err = msgp.ReadUint64Bytes(bts) + (*z).KeyregTxnFields.Nonparticipation, bts, err = msgp.ReadBoolBytes(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "KeyDilution") + err = msgp.WrapError(err, "struct-from-array", "Nonparticipation") + return + } + } + if zb0001 > 0 { + zb0001-- + bts, err = (*z).Parent.UnmarshalMsg(bts) + if err != nil { + err = msgp.WrapError(err, "struct-from-array", "Parent") return } } @@ -155,34 +189,46 @@ func (z *participationIDData) UnmarshalMsg(bts []byte) (o []byte, err error) { return } switch string(field) { - case "addr": - bts, err = (*z).Parent.UnmarshalMsg(bts) + case "votekey": + bts, err = (*z).KeyregTxnFields.VotePK.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "Parent") + err = msgp.WrapError(err, "VotePK") + return + } + case "selkey": + bts, err = (*z).KeyregTxnFields.SelectionPK.UnmarshalMsg(bts) + if err != nil { + err = msgp.WrapError(err, "SelectionPK") + return + } + case "votefst": + bts, err = (*z).KeyregTxnFields.VoteFirst.UnmarshalMsg(bts) + if err != nil { + err = msgp.WrapError(err, "VoteFirst") return } - case "vrfsk": - bts, err = (*z).VRFSK.UnmarshalMsg(bts) + case "votelst": + bts, err = (*z).KeyregTxnFields.VoteLast.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "VRFSK") + err = msgp.WrapError(err, "VoteLast") return } - case "fv": - bts, err = (*z).FirstValid.UnmarshalMsg(bts) + case "votekd": + (*z).KeyregTxnFields.VoteKeyDilution, bts, err = msgp.ReadUint64Bytes(bts) if err != nil { - err = msgp.WrapError(err, "FirstValid") + err = msgp.WrapError(err, "VoteKeyDilution") return } - case "lv": - bts, err = (*z).LastValid.UnmarshalMsg(bts) + case "nonpart": + (*z).KeyregTxnFields.Nonparticipation, bts, err = msgp.ReadBoolBytes(bts) if err != nil { - err = msgp.WrapError(err, "LastValid") + err = msgp.WrapError(err, "Nonparticipation") return } - case "kd": - (*z).KeyDilution, bts, err = msgp.ReadUint64Bytes(bts) + case "addr": + bts, err = (*z).Parent.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "KeyDilution") + err = msgp.WrapError(err, "Parent") return } default: @@ -205,11 +251,11 @@ func (_ *participationIDData) CanUnmarshalMsg(z interface{}) bool { // Msgsize returns an upper bound estimate of the number of bytes occupied by the serialized message func (z *participationIDData) Msgsize() (s int) { - s = 1 + 5 + (*z).Parent.Msgsize() + 6 + (*z).VRFSK.Msgsize() + 3 + (*z).FirstValid.Msgsize() + 3 + (*z).LastValid.Msgsize() + 3 + msgp.Uint64Size + s = 1 + 8 + (*z).KeyregTxnFields.VotePK.Msgsize() + 7 + (*z).KeyregTxnFields.SelectionPK.Msgsize() + 8 + (*z).KeyregTxnFields.VoteFirst.Msgsize() + 8 + (*z).KeyregTxnFields.VoteLast.Msgsize() + 7 + msgp.Uint64Size + 8 + msgp.BoolSize + 5 + (*z).Parent.Msgsize() return } // MsgIsZero returns whether this is a zero value func (z *participationIDData) MsgIsZero() bool { - return ((*z).Parent.MsgIsZero()) && ((*z).VRFSK.MsgIsZero()) && ((*z).FirstValid.MsgIsZero()) && ((*z).LastValid.MsgIsZero()) && ((*z).KeyDilution == 0) + return ((*z).KeyregTxnFields.VotePK.MsgIsZero()) && ((*z).KeyregTxnFields.SelectionPK.MsgIsZero()) && ((*z).KeyregTxnFields.VoteFirst.MsgIsZero()) && ((*z).KeyregTxnFields.VoteLast.MsgIsZero()) && ((*z).KeyregTxnFields.VoteKeyDilution == 0) && ((*z).KeyregTxnFields.Nonparticipation == false) && ((*z).Parent.MsgIsZero()) } diff --git a/data/account/msgp_gen_test.go b/data/account/msgp_gen_test.go index d66bf1fc86..5d25424499 100644 --- a/data/account/msgp_gen_test.go +++ b/data/account/msgp_gen_test.go @@ -8,12 +8,10 @@ import ( "testing" "github.com/algorand/go-algorand/protocol" - "github.com/algorand/go-algorand/test/partitiontest" "github.com/algorand/msgp/msgp" ) func TestMarshalUnmarshalparticipationIDData(t *testing.T) { - partitiontest.PartitionTest(t) v := participationIDData{} bts := v.MarshalMsg(nil) left, err := v.UnmarshalMsg(bts) @@ -34,7 +32,6 @@ func TestMarshalUnmarshalparticipationIDData(t *testing.T) { } func TestRandomizedEncodingparticipationIDData(t *testing.T) { - partitiontest.PartitionTest(t) protocol.RunEncodingTest(t, &participationIDData{}) } diff --git a/data/account/participation.go b/data/account/participation.go index 1a53f8aaae..184728571a 100644 --- a/data/account/participation.go +++ b/data/account/participation.go @@ -61,11 +61,8 @@ type Participation struct { type participationIDData struct { _struct struct{} `codec:",omitempty,omitemptyarray"` - Parent basics.Address `codec:"addr"` - VRFSK crypto.VrfPrivkey `codec:"vrfsk"` - FirstValid basics.Round `codec:"fv"` - LastValid basics.Round `codec:"lv"` - KeyDilution uint64 `codec:"kd"` + transactions.KeyregTxnFields + Parent basics.Address `codec:"addr"` } // ToBeHashed implements the Hashable interface. @@ -73,19 +70,27 @@ func (id *participationIDData) ToBeHashed() (protocol.HashID, []byte) { return protocol.ParticipationKeys, protocol.Encode(id) } +// MakeParticipationID generates the ParticipationID from an address and a key registration. +func MakeParticipationID(addr basics.Address, fields transactions.KeyregTxnFields) ParticipationID { + return ParticipationID(crypto.HashObj(&participationIDData{ + KeyregTxnFields: fields, + Parent: addr, + })) +} + // ParticipationID computes a ParticipationID. func (part Participation) ParticipationID() ParticipationID { - idData := participationIDData{ - Parent: part.Parent, - FirstValid: part.FirstValid, - LastValid: part.LastValid, - KeyDilution: part.KeyDilution, - } - if part.VRF != nil { - copy(idData.VRFSK[:], part.VRF.SK[:]) + // Allow a panic if Voting or VRF are not present. + fields := transactions.KeyregTxnFields{ + VotePK: part.Voting.OneTimeSignatureVerifier, + SelectionPK: part.VRF.PK, + VoteFirst: part.FirstValid, + VoteLast: part.LastValid, + VoteKeyDilution: part.KeyDilution, + Nonparticipation: false, } - return ParticipationID(crypto.HashObj(&idData)) + return MakeParticipationID(part.Parent, fields) } // PersistedParticipation encapsulates the static state of the participation diff --git a/data/account/participationRegistry_test.go b/data/account/participationRegistry_test.go index 542602db4f..4fc806f5ec 100644 --- a/data/account/participationRegistry_test.go +++ b/data/account/participationRegistry_test.go @@ -26,6 +26,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/algorand/go-algorand/crypto" "github.com/algorand/go-algorand/data/basics" "github.com/algorand/go-algorand/logging" "github.com/algorand/go-algorand/test/partitiontest" @@ -50,6 +51,18 @@ func assertParticipation(t *testing.T, p Participation, pr ParticipationRecord) require.Equal(t, p.Parent, pr.Account) } +func makeTestParticipation(addrID byte, first, last basics.Round, dilution uint64) Participation { + p := Participation{ + FirstValid: first, + LastValid: last, + KeyDilution: dilution, + Voting: &crypto.OneTimeSignatureSecrets{}, + VRF: &crypto.VRFSecrets{}, + } + p.Parent[0] = addrID + return p +} + // Insert participation records and make sure they can be fetched. func TestParticipation_InsertGet(t *testing.T) { partitiontest.PartitionTest(t) @@ -57,19 +70,8 @@ func TestParticipation_InsertGet(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 1, - LastValid: 2, - KeyDilution: 3, - } - p.Parent[0] = 1 - - p2 := Participation{ - FirstValid: 4, - LastValid: 5, - KeyDilution: 6, - } - p2.Parent[0] = 2 + p := makeTestParticipation(1, 1, 2, 3) + p2 := makeTestParticipation(2, 4, 5, 6) insertAndVerify := func(part Participation) { id, err := registry.Insert(part) @@ -108,19 +110,8 @@ func TestParticipation_Delete(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 1, - LastValid: 2, - KeyDilution: 3, - } - p.Parent[0] = 1 - - p2 := Participation{ - FirstValid: 4, - LastValid: 5, - KeyDilution: 6, - } - p2.Parent[0] = 2 + p := makeTestParticipation(1, 1, 2, 3) + p2 := makeTestParticipation(2, 4, 5, 6) id, err := registry.Insert(p) a.NoError(err) @@ -149,19 +140,8 @@ func TestParticipation_Register(t *testing.T) { defer registry.Close() // Overlapping keys. - p := Participation{ - FirstValid: 250000, - LastValid: 3000000, - KeyDilution: 1, - } - p.Parent[0] = 1 - - p2 := Participation{ - FirstValid: 2000000, - LastValid: 4000000, - KeyDilution: 2, - Parent: p.Parent, - } + p := makeTestParticipation(1, 250000, 3000000, 1) + p2 := makeTestParticipation(1, 200000, 4000000, 2) id, err := registry.Insert(p) a.NoError(err) @@ -197,11 +177,7 @@ func TestParticipation_RegisterInvalidID(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 250000, - LastValid: 3000000, - KeyDilution: 1, - } + p := makeTestParticipation(0, 250000, 3000000, 1) err := registry.Register(p.ParticipationID(), 10000000) a.EqualError(err, ErrParticipationIDNotFound.Error()) @@ -214,11 +190,7 @@ func TestParticipation_RegisterInvalidRange(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 250000, - LastValid: 3000000, - KeyDilution: 1, - } + p := makeTestParticipation(0, 250000, 3000000, 1) id, err := registry.Insert(p) a.NoError(err) @@ -237,18 +209,10 @@ func TestParticipation_Record(t *testing.T) { defer registry.Close() // Setup p - p := Participation{ - FirstValid: 0, - LastValid: 3000000, - KeyDilution: 1, - } - p.Parent[0] = 1 - + p := makeTestParticipation(1, 0, 3000000, 1) // Setup some other keys to make sure they are not updated. - p2 := p - p2.Parent[0] = 2 - p3 := p - p3.Parent[0] = 3 + p2 := makeTestParticipation(2, 0, 3000000, 1) + p3 := makeTestParticipation(3, 0, 3000000, 1) // Install and register all of the keys for _, part := range []Participation{p, p2, p3} { @@ -304,12 +268,7 @@ func TestParticipation_RecordInvalidActionAndOutOfRange(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 0, - LastValid: 3000000, - KeyDilution: 1, - } - p.Parent[0] = 1 + p := makeTestParticipation(1, 0, 3000000, 1) id, err := registry.Insert(p) a.NoError(err) err = registry.Register(id, 0) @@ -346,18 +305,8 @@ func TestParticipation_RecordMultipleUpdates(t *testing.T) { // We'll test that recording at this round fails because both keys are active testRound := basics.Round(5000) - p := Participation{ - FirstValid: 0, - LastValid: 3000000, - KeyDilution: 1, - } - p.Parent[0] = 1 - p2 := Participation{ - FirstValid: 1, - LastValid: 3000000, - KeyDilution: 1, - } - p2.Parent = p.Parent + p := makeTestParticipation(1, 0, 3000000, 1) + p2 := makeTestParticipation(1, 1, 3000000, 1) _, err := registry.Insert(p) a.NoError(err) @@ -403,12 +352,7 @@ func TestParticipation_MultipleInsertError(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 1, - LastValid: 2, - KeyDilution: 3, - } - p.Parent[0] = 1 + p := makeTestParticipation(1, 1, 2, 3) _, err := registry.Insert(p) a.NoError(err) @@ -427,12 +371,7 @@ func TestParticipation_RecordMultipleUpdates_DB(t *testing.T) { registry := getRegistry(t) defer registry.Close() - p := Participation{ - FirstValid: 1, - LastValid: 2000000, - KeyDilution: 3, - } - p.Parent[0] = 1 + p := makeTestParticipation(1, 1, 2000000, 3) id := p.ParticipationID() // Insert the same record twice diff --git a/node/node.go b/node/node.go index ec5309e826..d21f4e8f43 100644 --- a/node/node.go +++ b/node/node.go @@ -800,7 +800,7 @@ func (node *AlgorandFullNode) loadParticipationKeys() error { if err != nil { if db.IsErrBusy(err) { // this is a special case: - // we might get "database is locked" when we attempt to access a database that is conurrently updates it's participation keys. + // we might get "database is locked" when we attempt to access a database that is concurrently updating its participation keys. // that database is clearly already on the account manager, and doesn't need to be processed through this logic, and therefore // we can safely ignore that fail case. continue @@ -869,6 +869,18 @@ func (node *AlgorandFullNode) OnNewBlock(block bookkeeping.Block, delta ledgerco node.hasSyncedSinceStartup = true node.syncStatusMu.Unlock() + // Look for keyreg events and notify the participationRegistry. + for _, tx := range block.Payset { + if tx.Txn.Type == protocol.KeyRegistrationTx { + id := account.MakeParticipationID(tx.Txn.Sender, tx.Txn.KeyregTxnFields) + err := node.participationRegistry.Register(id, block.BlockHeader.Round) + // If the key is not installed on this node, ErrParticipationIDNotFound is quickly returned. + if err != nil && err != account.ErrParticipationIDNotFound { + node.log.Error("Problem with participationRegistry.Register: %w", err) + } + } + } + // Wake up oldKeyDeletionThread(), non-blocking. select { case node.oldKeyDeletionNotify <- struct{}{}: From ecaf450a6dee56074eb571dbd904c1f0a0f226f2 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Thu, 26 Aug 2021 12:43:51 -0400 Subject: [PATCH 02/11] Fix node test. --- node/node_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/node/node_test.go b/node/node_test.go index 9cb7f61ea7..6a7811e58f 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -536,6 +536,8 @@ func TestAsyncRecord(t *testing.T) { Parent: addr, FirstValid: 0, LastValid: 1000000, + Voting: &crypto.OneTimeSignatureSecrets{}, + VRF: &crypto.VRFSecrets{}, } id, err := node.participationRegistry.Insert(p) require.NoError(t, err) From a89833a3dd00e7c99c10fafaead5f44112b45516 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 10:45:49 -0400 Subject: [PATCH 03/11] Simplify key registration, cleanup ParticipationRegistry interface. --- data/account/participationRegistry.go | 69 ++++++++++++---------- data/account/participationRegistry_test.go | 60 +++++++------------ node/node.go | 18 ++---- 3 files changed, 65 insertions(+), 82 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index 7c612173c7..f0d5fdd133 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -46,13 +46,17 @@ type ParticipationRecord struct { LastVote basics.Round LastBlockProposal basics.Round LastCompactCertificate basics.Round - RegisteredFirst basics.Round - RegisteredLast basics.Round + EffectiveFirst basics.Round + EffectiveLast basics.Round // VRFSecrets // OneTimeSignatureSecrets } +func (r ParticipationRecord) IsZero() bool { + return r == (ParticipationRecord{}) +} + // Duplicate creates a copy of the current object. This is required once secrets are stored. func (r ParticipationRecord) Duplicate() ParticipationRecord { return ParticipationRecord{ @@ -64,8 +68,8 @@ func (r ParticipationRecord) Duplicate() ParticipationRecord { LastVote: r.LastVote, LastBlockProposal: r.LastBlockProposal, LastCompactCertificate: r.LastCompactCertificate, - RegisteredFirst: r.RegisteredFirst, - RegisteredLast: r.RegisteredLast, + EffectiveFirst: r.EffectiveFirst, + EffectiveLast: r.EffectiveLast, } } @@ -119,10 +123,10 @@ type ParticipationRegistry interface { Delete(id ParticipationID) error // Get a participation record. - Get(id ParticipationID) (ParticipationRecord, error) + Get(id ParticipationID) ParticipationRecord // GetAll of the participation records. - GetAll() ([]ParticipationRecord, error) + GetAll() []ParticipationRecord // Register updates the EffectiveFirst and EffectiveLast fields. If there are multiple records for the account // then it is possible for multiple records to be updated. @@ -192,8 +196,8 @@ var ( lastVoteRound INTEGER NOT NULL DEFAULT 0, lastBlockProposalRound INTEGER NOT NULL DEFAULT 0, lastCompactCertificateRound INTEGER NOT NULL DEFAULT 0, - registeredFirstRound INTEGER NOT NULL DEFAULT 0, - registeredLastRound INTEGER NOT NULL DEFAULT 0 + effectiveFirstRound INTEGER NOT NULL DEFAULT 0, + effectiveLastRound INTEGER NOT NULL DEFAULT 0 -- voting BLOB, --* msgpack encoding of ParticipationAccount.voting )` @@ -206,7 +210,7 @@ var ( selectRecords = `SELECT participationID, account, firstValidRound, lastValidRound, keyDilution, lastVoteRound, lastBlockProposalRound, lastCompactCertificateRound, - registeredFirstRound, registeredLastRound + effectiveFirstRound, effectiveLastRound FROM Keysets INNER JOIN Rolling ON Keysets.pk = Rolling.pk` @@ -216,8 +220,8 @@ var ( SET lastVoteRound=?, lastBlockProposalRound=?, lastCompactCertificateRound=?, - registeredFirstRound=?, - registeredLastRound=? + effectiveFirstRound=?, + effectiveLastRound=? WHERE pk IN (SELECT pk FROM Keysets WHERE participationID=?)` ) @@ -331,8 +335,8 @@ func (db *participationDB) Insert(record Participation) (id ParticipationID, err LastVote: 0, LastBlockProposal: 0, LastCompactCertificate: 0, - RegisteredFirst: 0, - RegisteredLast: 0, + EffectiveFirst: 0, + EffectiveLast: 0, } } @@ -395,8 +399,8 @@ func scanRecords(rows *sql.Rows) ([]ParticipationRecord, error) { &record.LastVote, &record.LastBlockProposal, &record.LastCompactCertificate, - &record.RegisteredFirst, - &record.RegisteredLast, + &record.EffectiveFirst, + &record.EffectiveLast, ) if err != nil { return nil, err @@ -430,18 +434,18 @@ func (db *participationDB) getAllFromDB() (records []ParticipationRecord, err er return } -func (db *participationDB) Get(id ParticipationID) (record ParticipationRecord, err error) { +func (db *participationDB) Get(id ParticipationID) ParticipationRecord { db.mutex.RLock() defer db.mutex.RUnlock() record, ok := db.cache[id] if !ok { - return ParticipationRecord{}, ErrParticipationIDNotFound + return ParticipationRecord{} } - return record.Duplicate(), nil + return record.Duplicate() } -func (db *participationDB) GetAll() ([]ParticipationRecord, error) { +func (db *participationDB) GetAll() []ParticipationRecord { db.mutex.RLock() defer db.mutex.RUnlock() @@ -449,7 +453,7 @@ func (db *participationDB) GetAll() ([]ParticipationRecord, error) { for _, record := range db.cache { results = append(results, record.Duplicate()) } - return results, nil + return results } // updateRollingFields sets all of the rolling fields according to the record object. @@ -458,8 +462,8 @@ func (db *participationDB) updateRollingFields(ctx context.Context, tx *sql.Tx, record.LastVote, record.LastBlockProposal, record.LastCompactCertificate, - record.RegisteredFirst, - record.RegisteredLast, + record.EffectiveFirst, + record.EffectiveLast, record.ParticipationID[:]) if err != nil { return err @@ -482,14 +486,14 @@ func (db *participationDB) updateRollingFields(ctx context.Context, tx *sql.Tx, } func recordActive(record ParticipationRecord, on basics.Round) bool { - return record.RegisteredFirst <= on && on <= record.RegisteredLast + return record.EffectiveLast != 0 && record.EffectiveFirst <= on && on <= record.EffectiveLast } func (db *participationDB) Register(id ParticipationID, on basics.Round) error { // Lookup recordToRegister for first/last valid and account. - recordToRegister, err := db.Get(id) - if err != nil { - return err + recordToRegister := db.Get(id) + if recordToRegister.IsZero() { + return ErrParticipationIDNotFound } // round out of valid range. @@ -497,13 +501,18 @@ func (db *participationDB) Register(id ParticipationID, on basics.Round) error { return ErrInvalidRegisterRange } + // No-op If the record is already active + if recordActive(recordToRegister, on) { + return nil + } + updated := make(map[ParticipationID]ParticipationRecord) - err = db.store.Wdb.Atomic(func(ctx context.Context, tx *sql.Tx) error { + err := db.store.Wdb.Atomic(func(ctx context.Context, tx *sql.Tx) error { // Disable active key if there is one for _, record := range db.cache { if record.Account == recordToRegister.Account && record.ParticipationID != id && recordActive(record, on) { // TODO: this should probably be "on - 1" - record.RegisteredLast = on + record.EffectiveLast = on err := db.updateRollingFields(ctx, tx, record) // Repair the case when no keys were updated if err == ErrNoKeyForID { @@ -519,8 +528,8 @@ func (db *participationDB) Register(id ParticipationID, on basics.Round) error { } // Mark registered. - recordToRegister.RegisteredFirst = on - recordToRegister.RegisteredLast = recordToRegister.LastValid + recordToRegister.EffectiveFirst = on + recordToRegister.EffectiveLast = recordToRegister.LastValid err := db.updateRollingFields(ctx, tx, recordToRegister) if err == ErrNoKeyForID { diff --git a/data/account/participationRegistry_test.go b/data/account/participationRegistry_test.go index 4fc806f5ec..9a0d31b951 100644 --- a/data/account/participationRegistry_test.go +++ b/data/account/participationRegistry_test.go @@ -19,7 +19,6 @@ package account import ( "context" "database/sql" - "errors" "fmt" "testing" @@ -78,8 +77,8 @@ func TestParticipation_InsertGet(t *testing.T) { a.NoError(err) a.Equal(part.ParticipationID(), id) - record, err := registry.Get(part.ParticipationID()) - a.NoError(err) + record := registry.Get(part.ParticipationID()) + a.False(record.IsZero()) assertParticipation(t, part, record) } @@ -89,8 +88,7 @@ func TestParticipation_InsertGet(t *testing.T) { // Data should be persisted immediately, re-initialize cache and verify GetAll. registry.initializeCache() - results, err := registry.GetAll() - a.NoError(err) + results := registry.GetAll() a.Len(results, 2) for _, record := range results { if record.Account == p.Parent { @@ -126,8 +124,7 @@ func TestParticipation_Delete(t *testing.T) { // Delete should be persisted immediately. Verify p removed in GetAll. registry.initializeCache() - results, err := registry.GetAll() - a.NoError(err) + results := registry.GetAll() a.Len(results, 1) assertParticipation(t, p2, results[0]) } @@ -152,10 +149,10 @@ func TestParticipation_Register(t *testing.T) { a.Equal(p2.ParticipationID(), id) verifyEffectiveRound := func(id ParticipationID, first, last int) { - record, err := registry.Get(id) - a.NoError(err) - require.Equal(t, first, int(record.RegisteredFirst)) - require.Equal(t, last, int(record.RegisteredLast)) + record := registry.Get(id) + a.False(record.IsZero()) + require.Equal(t, first, int(record.EffectiveFirst)) + require.Equal(t, last, int(record.EffectiveLast)) } // Register the first key. @@ -223,11 +220,10 @@ func TestParticipation_Record(t *testing.T) { a.NoError(err) } - all, err := registry.GetAll() + all := registry.GetAll() a.NotNil(all) - a.NoError(err) - err = registry.Record(p.Parent, 1000, Vote) + err := registry.Record(p.Parent, 1000, Vote) a.NoError(err) err = registry.Record(p.Parent, 2000, BlockProposal) a.NoError(err) @@ -236,8 +232,7 @@ func TestParticipation_Record(t *testing.T) { // Verify that one and only one key was updated. test := func(registry ParticipationRegistry) { - records, err := registry.GetAll() - a.NoError(err) + records := registry.GetAll() a.Len(records, 3) for _, record := range records { if record.ParticipationID == p.ParticipationID() { @@ -317,8 +312,8 @@ func TestParticipation_RecordMultipleUpdates(t *testing.T) { // Force the DB to have 2 active keys for one account by tampering with the private cache variable recordCopy := registry.cache[p2.ParticipationID()] - recordCopy.RegisteredFirst = p2.FirstValid - recordCopy.RegisteredLast = p2.LastValid + recordCopy.EffectiveFirst = p2.FirstValid + recordCopy.EffectiveLast = p2.LastValid registry.cache[p2.ParticipationID()] = recordCopy registry.dirty[p2.ParticipationID()] = struct{}{} registry.Flush() @@ -329,12 +324,12 @@ func TestParticipation_RecordMultipleUpdates(t *testing.T) { a.NotEqual(p.ParticipationID(), p2.ParticipationID()) recordTest := make([]ParticipationRecord, 0) - recordP, err := registry.Get(p.ParticipationID()) - a.NoError(err) + recordP := registry.Get(p.ParticipationID()) + a.False(recordP.IsZero()) recordTest = append(recordTest, recordP) - recordP2, err := registry.Get(p2.ParticipationID()) - a.NoError(err) + recordP2 := registry.Get(p2.ParticipationID()) + a.False(recordP2.IsZero()) recordTest = append(recordTest, recordP2) // Make sure both accounts are active for the test round @@ -399,7 +394,7 @@ func TestParticipation_RecordMultipleUpdates_DB(t *testing.T) { } // Create Rolling entry - _, err = tx.Exec(`INSERT INTO Rolling (pk, registeredFirstRound, registeredLastRound) VALUES (?, ?, ?)`, pk, 1, 2000000) + _, err = tx.Exec(`INSERT INTO Rolling (pk, effectiveFirstRound, effectiveLastRound) VALUES (?, ?, ?)`, pk, 1, 2000000) if err != nil { return fmt.Errorf("unable insert rolling: %w", err) } @@ -423,21 +418,6 @@ func TestParticipation_RecordMultipleUpdates_DB(t *testing.T) { err = registry.initializeCache() a.EqualError(err, ErrMultipleKeysForID.Error()) - // Registering the ID - registry.cache[id] = ParticipationRecord{ - ParticipationID: id, - Account: p.Parent, - FirstValid: p.FirstValid, - LastValid: p.LastValid, - KeyDilution: p.KeyDilution, - RegisteredFirst: p.FirstValid, - RegisteredLast: p.LastValid, - } - err = registry.Register(id, 1) - a.Error(err) - a.Contains(err.Error(), "unable to registering key with id") - a.EqualError(errors.Unwrap(err), ErrMultipleKeysForID.Error()) - // Flushing changes detects that multiple records are updated registry.dirty[id] = struct{}{} err = registry.Flush() @@ -458,8 +438,8 @@ func TestParticipation_NoKeyToUpdate(t *testing.T) { FirstValid: 1, LastValid: 2, KeyDilution: 3, - RegisteredFirst: 4, - RegisteredLast: 5, + EffectiveFirst: 4, + EffectiveLast: 5, } err := registry.updateRollingFields(ctx, tx, record) a.EqualError(err, ErrNoKeyForID.Error()) diff --git a/node/node.go b/node/node.go index d21f4e8f43..dd22e325d9 100644 --- a/node/node.go +++ b/node/node.go @@ -869,18 +869,6 @@ func (node *AlgorandFullNode) OnNewBlock(block bookkeeping.Block, delta ledgerco node.hasSyncedSinceStartup = true node.syncStatusMu.Unlock() - // Look for keyreg events and notify the participationRegistry. - for _, tx := range block.Payset { - if tx.Txn.Type == protocol.KeyRegistrationTx { - id := account.MakeParticipationID(tx.Txn.Sender, tx.Txn.KeyregTxnFields) - err := node.participationRegistry.Register(id, block.BlockHeader.Round) - // If the key is not installed on this node, ErrParticipationIDNotFound is quickly returned. - if err != nil && err != account.ErrParticipationIDNotFound { - node.log.Error("Problem with participationRegistry.Register: %w", err) - } - } - } - // Wake up oldKeyDeletionThread(), non-blocking. select { case node.oldKeyDeletionNotify <- struct{}{}: @@ -1173,6 +1161,12 @@ func (node *AlgorandFullNode) VotingKeys(votingRound, keysRound basics.Round) [] } participations = append(participations, part) matchingAccountsKeys[part.Address()] = true + + // This is usually a no-op, but the first time it will update the DB. + err := node.participationRegistry.Register(part.ParticipationID(), keysRound) + if err != nil { + node.log.Error("Failed to register participation key (%s) with participation registry.", part.ParticipationID()) + } } // write the warnings per account only if we couldn't find a single valid key for that account. for mismatchingAddr, warningFlags := range mismatchingAccountsKeys { From aedceb271c844f911fdfd1e7438428c4d91ff7dc Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 10:50:41 -0400 Subject: [PATCH 04/11] Revert participationIDData change. --- data/account/msgp_gen.go | 148 +++++++++----------------- data/account/participation.go | 33 +++--- data/account/participationRegistry.go | 1 + 3 files changed, 66 insertions(+), 116 deletions(-) diff --git a/data/account/msgp_gen.go b/data/account/msgp_gen.go index 6df260ec8f..3c2370f167 100644 --- a/data/account/msgp_gen.go +++ b/data/account/msgp_gen.go @@ -20,73 +20,55 @@ import ( func (z *participationIDData) MarshalMsg(b []byte) (o []byte) { o = msgp.Require(b, z.Msgsize()) // omitempty: check for empty values - zb0001Len := uint32(7) - var zb0001Mask uint16 /* 9 bits */ + zb0001Len := uint32(5) + var zb0001Mask uint8 /* 6 bits */ if (*z).Parent.MsgIsZero() { + zb0001Len-- + zb0001Mask |= 0x2 + } + if (*z).FirstValid.MsgIsZero() { zb0001Len-- zb0001Mask |= 0x4 } - if (*z).KeyregTxnFields.Nonparticipation == false { + if (*z).KeyDilution == 0 { zb0001Len-- zb0001Mask |= 0x8 } - if (*z).KeyregTxnFields.SelectionPK.MsgIsZero() { + if (*z).LastValid.MsgIsZero() { zb0001Len-- zb0001Mask |= 0x10 } - if (*z).KeyregTxnFields.VoteFirst.MsgIsZero() { + if (*z).VRFSK.MsgIsZero() { zb0001Len-- zb0001Mask |= 0x20 } - if (*z).KeyregTxnFields.VoteKeyDilution == 0 { - zb0001Len-- - zb0001Mask |= 0x40 - } - if (*z).KeyregTxnFields.VotePK.MsgIsZero() { - zb0001Len-- - zb0001Mask |= 0x80 - } - if (*z).KeyregTxnFields.VoteLast.MsgIsZero() { - zb0001Len-- - zb0001Mask |= 0x100 - } // variable map header, size zb0001Len o = append(o, 0x80|uint8(zb0001Len)) if zb0001Len != 0 { - if (zb0001Mask & 0x4) == 0 { // if not empty + if (zb0001Mask & 0x2) == 0 { // if not empty // string "addr" o = append(o, 0xa4, 0x61, 0x64, 0x64, 0x72) o = (*z).Parent.MarshalMsg(o) } + if (zb0001Mask & 0x4) == 0 { // if not empty + // string "fv" + o = append(o, 0xa2, 0x66, 0x76) + o = (*z).FirstValid.MarshalMsg(o) + } if (zb0001Mask & 0x8) == 0 { // if not empty - // string "nonpart" - o = append(o, 0xa7, 0x6e, 0x6f, 0x6e, 0x70, 0x61, 0x72, 0x74) - o = msgp.AppendBool(o, (*z).KeyregTxnFields.Nonparticipation) + // string "kd" + o = append(o, 0xa2, 0x6b, 0x64) + o = msgp.AppendUint64(o, (*z).KeyDilution) } if (zb0001Mask & 0x10) == 0 { // if not empty - // string "selkey" - o = append(o, 0xa6, 0x73, 0x65, 0x6c, 0x6b, 0x65, 0x79) - o = (*z).KeyregTxnFields.SelectionPK.MarshalMsg(o) + // string "lv" + o = append(o, 0xa2, 0x6c, 0x76) + o = (*z).LastValid.MarshalMsg(o) } if (zb0001Mask & 0x20) == 0 { // if not empty - // string "votefst" - o = append(o, 0xa7, 0x76, 0x6f, 0x74, 0x65, 0x66, 0x73, 0x74) - o = (*z).KeyregTxnFields.VoteFirst.MarshalMsg(o) - } - if (zb0001Mask & 0x40) == 0 { // if not empty - // string "votekd" - o = append(o, 0xa6, 0x76, 0x6f, 0x74, 0x65, 0x6b, 0x64) - o = msgp.AppendUint64(o, (*z).KeyregTxnFields.VoteKeyDilution) - } - if (zb0001Mask & 0x80) == 0 { // if not empty - // string "votekey" - o = append(o, 0xa7, 0x76, 0x6f, 0x74, 0x65, 0x6b, 0x65, 0x79) - o = (*z).KeyregTxnFields.VotePK.MarshalMsg(o) - } - if (zb0001Mask & 0x100) == 0 { // if not empty - // string "votelst" - o = append(o, 0xa7, 0x76, 0x6f, 0x74, 0x65, 0x6c, 0x73, 0x74) - o = (*z).KeyregTxnFields.VoteLast.MarshalMsg(o) + // string "vrfsk" + o = append(o, 0xa5, 0x76, 0x72, 0x66, 0x73, 0x6b) + o = (*z).VRFSK.MarshalMsg(o) } } return @@ -112,57 +94,41 @@ func (z *participationIDData) UnmarshalMsg(bts []byte) (o []byte, err error) { } if zb0001 > 0 { zb0001-- - bts, err = (*z).KeyregTxnFields.VotePK.UnmarshalMsg(bts) - if err != nil { - err = msgp.WrapError(err, "struct-from-array", "VotePK") - return - } - } - if zb0001 > 0 { - zb0001-- - bts, err = (*z).KeyregTxnFields.SelectionPK.UnmarshalMsg(bts) - if err != nil { - err = msgp.WrapError(err, "struct-from-array", "SelectionPK") - return - } - } - if zb0001 > 0 { - zb0001-- - bts, err = (*z).KeyregTxnFields.VoteFirst.UnmarshalMsg(bts) + bts, err = (*z).Parent.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "VoteFirst") + err = msgp.WrapError(err, "struct-from-array", "Parent") return } } if zb0001 > 0 { zb0001-- - bts, err = (*z).KeyregTxnFields.VoteLast.UnmarshalMsg(bts) + bts, err = (*z).VRFSK.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "VoteLast") + err = msgp.WrapError(err, "struct-from-array", "VRFSK") return } } if zb0001 > 0 { zb0001-- - (*z).KeyregTxnFields.VoteKeyDilution, bts, err = msgp.ReadUint64Bytes(bts) + bts, err = (*z).FirstValid.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "VoteKeyDilution") + err = msgp.WrapError(err, "struct-from-array", "FirstValid") return } } if zb0001 > 0 { zb0001-- - (*z).KeyregTxnFields.Nonparticipation, bts, err = msgp.ReadBoolBytes(bts) + bts, err = (*z).LastValid.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "Nonparticipation") + err = msgp.WrapError(err, "struct-from-array", "LastValid") return } } if zb0001 > 0 { zb0001-- - bts, err = (*z).Parent.UnmarshalMsg(bts) + (*z).KeyDilution, bts, err = msgp.ReadUint64Bytes(bts) if err != nil { - err = msgp.WrapError(err, "struct-from-array", "Parent") + err = msgp.WrapError(err, "struct-from-array", "KeyDilution") return } } @@ -189,46 +155,34 @@ func (z *participationIDData) UnmarshalMsg(bts []byte) (o []byte, err error) { return } switch string(field) { - case "votekey": - bts, err = (*z).KeyregTxnFields.VotePK.UnmarshalMsg(bts) - if err != nil { - err = msgp.WrapError(err, "VotePK") - return - } - case "selkey": - bts, err = (*z).KeyregTxnFields.SelectionPK.UnmarshalMsg(bts) - if err != nil { - err = msgp.WrapError(err, "SelectionPK") - return - } - case "votefst": - bts, err = (*z).KeyregTxnFields.VoteFirst.UnmarshalMsg(bts) + case "addr": + bts, err = (*z).Parent.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "VoteFirst") + err = msgp.WrapError(err, "Parent") return } - case "votelst": - bts, err = (*z).KeyregTxnFields.VoteLast.UnmarshalMsg(bts) + case "vrfsk": + bts, err = (*z).VRFSK.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "VoteLast") + err = msgp.WrapError(err, "VRFSK") return } - case "votekd": - (*z).KeyregTxnFields.VoteKeyDilution, bts, err = msgp.ReadUint64Bytes(bts) + case "fv": + bts, err = (*z).FirstValid.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "VoteKeyDilution") + err = msgp.WrapError(err, "FirstValid") return } - case "nonpart": - (*z).KeyregTxnFields.Nonparticipation, bts, err = msgp.ReadBoolBytes(bts) + case "lv": + bts, err = (*z).LastValid.UnmarshalMsg(bts) if err != nil { - err = msgp.WrapError(err, "Nonparticipation") + err = msgp.WrapError(err, "LastValid") return } - case "addr": - bts, err = (*z).Parent.UnmarshalMsg(bts) + case "kd": + (*z).KeyDilution, bts, err = msgp.ReadUint64Bytes(bts) if err != nil { - err = msgp.WrapError(err, "Parent") + err = msgp.WrapError(err, "KeyDilution") return } default: @@ -251,11 +205,11 @@ func (_ *participationIDData) CanUnmarshalMsg(z interface{}) bool { // Msgsize returns an upper bound estimate of the number of bytes occupied by the serialized message func (z *participationIDData) Msgsize() (s int) { - s = 1 + 8 + (*z).KeyregTxnFields.VotePK.Msgsize() + 7 + (*z).KeyregTxnFields.SelectionPK.Msgsize() + 8 + (*z).KeyregTxnFields.VoteFirst.Msgsize() + 8 + (*z).KeyregTxnFields.VoteLast.Msgsize() + 7 + msgp.Uint64Size + 8 + msgp.BoolSize + 5 + (*z).Parent.Msgsize() + s = 1 + 5 + (*z).Parent.Msgsize() + 6 + (*z).VRFSK.Msgsize() + 3 + (*z).FirstValid.Msgsize() + 3 + (*z).LastValid.Msgsize() + 3 + msgp.Uint64Size return } // MsgIsZero returns whether this is a zero value func (z *participationIDData) MsgIsZero() bool { - return ((*z).KeyregTxnFields.VotePK.MsgIsZero()) && ((*z).KeyregTxnFields.SelectionPK.MsgIsZero()) && ((*z).KeyregTxnFields.VoteFirst.MsgIsZero()) && ((*z).KeyregTxnFields.VoteLast.MsgIsZero()) && ((*z).KeyregTxnFields.VoteKeyDilution == 0) && ((*z).KeyregTxnFields.Nonparticipation == false) && ((*z).Parent.MsgIsZero()) + return ((*z).Parent.MsgIsZero()) && ((*z).VRFSK.MsgIsZero()) && ((*z).FirstValid.MsgIsZero()) && ((*z).LastValid.MsgIsZero()) && ((*z).KeyDilution == 0) } diff --git a/data/account/participation.go b/data/account/participation.go index 184728571a..1a53f8aaae 100644 --- a/data/account/participation.go +++ b/data/account/participation.go @@ -61,8 +61,11 @@ type Participation struct { type participationIDData struct { _struct struct{} `codec:",omitempty,omitemptyarray"` - transactions.KeyregTxnFields - Parent basics.Address `codec:"addr"` + Parent basics.Address `codec:"addr"` + VRFSK crypto.VrfPrivkey `codec:"vrfsk"` + FirstValid basics.Round `codec:"fv"` + LastValid basics.Round `codec:"lv"` + KeyDilution uint64 `codec:"kd"` } // ToBeHashed implements the Hashable interface. @@ -70,27 +73,19 @@ func (id *participationIDData) ToBeHashed() (protocol.HashID, []byte) { return protocol.ParticipationKeys, protocol.Encode(id) } -// MakeParticipationID generates the ParticipationID from an address and a key registration. -func MakeParticipationID(addr basics.Address, fields transactions.KeyregTxnFields) ParticipationID { - return ParticipationID(crypto.HashObj(&participationIDData{ - KeyregTxnFields: fields, - Parent: addr, - })) -} - // ParticipationID computes a ParticipationID. func (part Participation) ParticipationID() ParticipationID { - // Allow a panic if Voting or VRF are not present. - fields := transactions.KeyregTxnFields{ - VotePK: part.Voting.OneTimeSignatureVerifier, - SelectionPK: part.VRF.PK, - VoteFirst: part.FirstValid, - VoteLast: part.LastValid, - VoteKeyDilution: part.KeyDilution, - Nonparticipation: false, + idData := participationIDData{ + Parent: part.Parent, + FirstValid: part.FirstValid, + LastValid: part.LastValid, + KeyDilution: part.KeyDilution, + } + if part.VRF != nil { + copy(idData.VRFSK[:], part.VRF.SK[:]) } - return MakeParticipationID(part.Parent, fields) + return ParticipationID(crypto.HashObj(&idData)) } // PersistedParticipation encapsulates the static state of the participation diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index f0d5fdd133..e461d70f30 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -53,6 +53,7 @@ type ParticipationRecord struct { // OneTimeSignatureSecrets } +// IsZero returns true if the object contains zero values. func (r ParticipationRecord) IsZero() bool { return r == (ParticipationRecord{}) } From d0bf74e62f7735cde3ab655ca9a4335106616c1a Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 10:51:50 -0400 Subject: [PATCH 05/11] Add partitiontest back --- data/account/msgp_gen_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/data/account/msgp_gen_test.go b/data/account/msgp_gen_test.go index 5d25424499..d66bf1fc86 100644 --- a/data/account/msgp_gen_test.go +++ b/data/account/msgp_gen_test.go @@ -8,10 +8,12 @@ import ( "testing" "github.com/algorand/go-algorand/protocol" + "github.com/algorand/go-algorand/test/partitiontest" "github.com/algorand/msgp/msgp" ) func TestMarshalUnmarshalparticipationIDData(t *testing.T) { + partitiontest.PartitionTest(t) v := participationIDData{} bts := v.MarshalMsg(nil) left, err := v.UnmarshalMsg(bts) @@ -32,6 +34,7 @@ func TestMarshalUnmarshalparticipationIDData(t *testing.T) { } func TestRandomizedEncodingparticipationIDData(t *testing.T) { + partitiontest.PartitionTest(t) protocol.RunEncodingTest(t, &participationIDData{}) } From 5972f4a54840921491d4e5f37c88b689c1e651a5 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 11:19:38 -0400 Subject: [PATCH 06/11] Improve DB in inconsistent state tests. --- data/account/participationRegistry.go | 14 ++++++-- data/account/participationRegistry_test.go | 39 ++++++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index e461d70f30..8591e6d763 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -21,6 +21,7 @@ import ( "database/sql" "errors" "fmt" + "strings" "github.com/algorand/go-deadlock" @@ -606,7 +607,7 @@ func (db *participationDB) Flush() error { // Verify that the dirty flag has not desynchronized from the cache. for id := range db.dirty { if _, ok := db.cache[id]; !ok { - db.log.Warn("participationDB fixing dirty flag de-synchronization for %s", id) + db.log.Warnf("participationDB fixing dirty flag de-synchronization for %s", id) delete(db.cache, id) } } @@ -616,13 +617,20 @@ func (db *participationDB) Flush() error { } err := db.store.Wdb.Atomic(func(ctx context.Context, tx *sql.Tx) error { + var errorStr strings.Builder for id := range db.dirty { err := db.updateRollingFields(ctx, tx, db.cache[id]) // This should only be updating key usage so ignoring missing keys is not a problem. if err != nil && err != ErrNoKeyForID { - return err + if errorStr.Len() > 0 { + errorStr.WriteString(", ") + } + errorStr.WriteString(err.Error()) } } + if errorStr.Len() > 0 { + return errors.New(errorStr.String()) + } return nil }) @@ -636,7 +644,7 @@ func (db *participationDB) Flush() error { func (db *participationDB) Close() { if err := db.Flush(); err != nil { - db.log.Warn("participationDB unhandled error during Close/Flush: %w", err) + db.log.Warnf("participationDB unhandled error during Close/Flush: %w", err) } db.store.Close() diff --git a/data/account/participationRegistry_test.go b/data/account/participationRegistry_test.go index 9a0d31b951..d30731c6bb 100644 --- a/data/account/participationRegistry_test.go +++ b/data/account/participationRegistry_test.go @@ -19,7 +19,9 @@ package account import ( "context" "database/sql" + "errors" "fmt" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -364,7 +366,6 @@ func TestParticipation_RecordMultipleUpdates_DB(t *testing.T) { partitiontest.PartitionTest(t) a := require.New(t) registry := getRegistry(t) - defer registry.Close() p := makeTestParticipation(1, 1, 2000000, 3) id := p.ParticipationID() @@ -394,7 +395,7 @@ func TestParticipation_RecordMultipleUpdates_DB(t *testing.T) { } // Create Rolling entry - _, err = tx.Exec(`INSERT INTO Rolling (pk, effectiveFirstRound, effectiveLastRound) VALUES (?, ?, ?)`, pk, 1, 2000000) + _, err = tx.Exec(`INSERT INTO Rolling (pk, effectiveFirstRound, effectiveLastRound) VALUES (?, ?, ?)`, pk, 1, 200000) if err != nil { return fmt.Errorf("unable insert rolling: %w", err) } @@ -418,11 +419,45 @@ func TestParticipation_RecordMultipleUpdates_DB(t *testing.T) { err = registry.initializeCache() a.EqualError(err, ErrMultipleKeysForID.Error()) + // Registering the ID - No error because it is already registered so we don't try to re-register. + registry.cache[id] = ParticipationRecord{ + ParticipationID: id, + Account: p.Parent, + FirstValid: p.FirstValid, + LastValid: p.LastValid, + KeyDilution: p.KeyDilution, + EffectiveFirst: p.FirstValid, + EffectiveLast: p.LastValid, + } + err = registry.Register(id, 1) + a.NoError(err) + + // Clear the first/last so that the no-op registration can't be detected + record := registry.cache[id] + record.EffectiveFirst = 0 + record.EffectiveLast = 0 + registry.cache[id] = record + + err = registry.Register(id, 1) + a.Error(err) + a.Contains(err.Error(), "unable to registering key with id") + a.EqualError(errors.Unwrap(err), ErrMultipleKeysForID.Error()) + // Flushing changes detects that multiple records are updated registry.dirty[id] = struct{}{} err = registry.Flush() a.Len(registry.dirty, 1) a.EqualError(err, ErrMultipleKeysForID.Error()) + + err = registry.Flush() + a.EqualError(err, ErrMultipleKeysForID.Error()) + + // Make sure the error message is logged when closing the registry. + var logOutput strings.Builder + registry.log.SetOutput(&logOutput) + registry.Close() + a.Contains(logOutput.String(), "participationDB unhandled error during Close/Flush") + a.Contains(logOutput.String(), ErrMultipleKeysForID.Error()) } func TestParticipation_NoKeyToUpdate(t *testing.T) { From 8bd7c838b9f839fafd0d5bda87ae0714dfba678b Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 12:18:21 -0400 Subject: [PATCH 07/11] Fix node test. --- data/account/participationRegistry_test.go | 59 +++++++++++++++++++++- logging/testingLogger.go | 2 +- 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/data/account/participationRegistry_test.go b/data/account/participationRegistry_test.go index d30731c6bb..691f31c9c4 100644 --- a/data/account/participationRegistry_test.go +++ b/data/account/participationRegistry_test.go @@ -19,6 +19,7 @@ package account import ( "context" "database/sql" + "encoding/binary" "errors" "fmt" "strings" @@ -52,7 +53,7 @@ func assertParticipation(t *testing.T, p Participation, pr ParticipationRecord) require.Equal(t, p.Parent, pr.Account) } -func makeTestParticipation(addrID byte, first, last basics.Round, dilution uint64) Participation { +func makeTestParticipation(addrID int, first, last basics.Round, dilution uint64) Participation { p := Participation{ FirstValid: first, LastValid: last, @@ -60,7 +61,7 @@ func makeTestParticipation(addrID byte, first, last basics.Round, dilution uint6 Voting: &crypto.OneTimeSignatureSecrets{}, VRF: &crypto.VRFSecrets{}, } - p.Parent[0] = addrID + binary.LittleEndian.PutUint32(p.Parent[:], uint32(addrID)) return p } @@ -481,3 +482,57 @@ func TestParticipation_NoKeyToUpdate(t *testing.T) { return nil }) } + +func benchmarkKeyRegistration(numKeys int, b *testing.B) { + // setup + rootDB, err := db.OpenPair(b.Name(), true) + if err != nil { + b.Fail() + } + registry, err := makeParticipationRegistry(rootDB, logging.TestingLog(b)) + if err != nil { + b.Fail() + } + + // Insert records so that we can t + b.Run("KeyInsert", func(b *testing.B) { + for n := 0; n < b.N; n++ { + for key := 0; key < numKeys; key++ { + p := makeTestParticipation(key, basics.Round(0), basics.Round(1000000), 3) + registry.Insert(p) + } + } + }) + + // The first call to Register updates the DB. + b.Run("KeyRegistered", func(b *testing.B) { + for n := 0; n < b.N; n++ { + for key := 0; key < numKeys; key++ { + p := makeTestParticipation(key, basics.Round(0), basics.Round(1000000), 3) + + // Unfortunately we need to repeatedly clear out the registration fields to ensure the + // db update runs each time this is called. + record := registry.cache[p.ParticipationID()] + record.EffectiveFirst = 0 + record.EffectiveLast = 0 + registry.cache[p.ParticipationID()] = record + registry.Register(p.ParticipationID(), 50) + } + } + }) + + // The keys should now be updated, so Register is a no-op. + b.Run("NoOp", func(b *testing.B) { + for n := 0; n < b.N; n++ { + for key := 0; key < numKeys; key++ { + p := makeTestParticipation(key, basics.Round(0), basics.Round(1000000), 3) + registry.Register(p.ParticipationID(), 50) + } + } + }) +} + +func BenchmarkKeyRegistration1(b *testing.B) { benchmarkKeyRegistration(1, b) } +func BenchmarkKeyRegistration5(b *testing.B) { benchmarkKeyRegistration(5, b) } +func BenchmarkKeyRegistration10(b *testing.B) { benchmarkKeyRegistration(10, b) } +func BenchmarkKeyRegistration50(b *testing.B) { benchmarkKeyRegistration(50, b) } diff --git a/logging/testingLogger.go b/logging/testingLogger.go index bbdb0f32a9..09b789fb07 100644 --- a/logging/testingLogger.go +++ b/logging/testingLogger.go @@ -22,7 +22,7 @@ import ( // TestLogWriter is an io.Writer that wraps a testing.T (or a testing.B) -- anything written to it gets logged with t.Log(...) // Being an io.Writer lets us pass it to Logger.SetOutput() in testing code -- this way if we want we can use Go's built-in testing log instead of making a new base.log file for each test. -// As a bonus, the detailed logs produced in a Travis test are now easily accessible and are printed if and only if that particular ttest fails. +// As a bonus, the detailed logs produced in a Travis test are now easily accessible and are printed if and only if that particular test fails. type TestLogWriter struct { testing.TB } From f4dad58a3f56ad13673d34ed0f59f4bea792fa2c Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 12:40:21 -0400 Subject: [PATCH 08/11] Fix node test and update benchmark. --- data/account/participationRegistry.go | 4 +++- data/account/participationRegistry_test.go | 10 +++++----- node/node_test.go | 3 +-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index 8591e6d763..5f0e870875 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -54,9 +54,11 @@ type ParticipationRecord struct { // OneTimeSignatureSecrets } +var zeroParticipationRecord = ParticipationRecord{} + // IsZero returns true if the object contains zero values. func (r ParticipationRecord) IsZero() bool { - return r == (ParticipationRecord{}) + return r == zeroParticipationRecord } // Duplicate creates a copy of the current object. This is required once secrets are stored. diff --git a/data/account/participationRegistry_test.go b/data/account/participationRegistry_test.go index 691f31c9c4..05cc268831 100644 --- a/data/account/participationRegistry_test.go +++ b/data/account/participationRegistry_test.go @@ -495,7 +495,7 @@ func benchmarkKeyRegistration(numKeys int, b *testing.B) { } // Insert records so that we can t - b.Run("KeyInsert", func(b *testing.B) { + b.Run(fmt.Sprintf("KeyInsert_%d", numKeys), func(b *testing.B) { for n := 0; n < b.N; n++ { for key := 0; key < numKeys; key++ { p := makeTestParticipation(key, basics.Round(0), basics.Round(1000000), 3) @@ -505,7 +505,7 @@ func benchmarkKeyRegistration(numKeys int, b *testing.B) { }) // The first call to Register updates the DB. - b.Run("KeyRegistered", func(b *testing.B) { + b.Run(fmt.Sprintf("KeyRegistered_%d", numKeys), func(b *testing.B) { for n := 0; n < b.N; n++ { for key := 0; key < numKeys; key++ { p := makeTestParticipation(key, basics.Round(0), basics.Round(1000000), 3) @@ -522,7 +522,7 @@ func benchmarkKeyRegistration(numKeys int, b *testing.B) { }) // The keys should now be updated, so Register is a no-op. - b.Run("NoOp", func(b *testing.B) { + b.Run(fmt.Sprintf("NoOp_%d", numKeys), func(b *testing.B) { for n := 0; n < b.N; n++ { for key := 0; key < numKeys; key++ { p := makeTestParticipation(key, basics.Round(0), basics.Round(1000000), 3) @@ -532,7 +532,7 @@ func benchmarkKeyRegistration(numKeys int, b *testing.B) { }) } -func BenchmarkKeyRegistration1(b *testing.B) { benchmarkKeyRegistration(1, b) } -func BenchmarkKeyRegistration5(b *testing.B) { benchmarkKeyRegistration(5, b) } +func BenchmarkKeyRegistration1(b *testing.B) { benchmarkKeyRegistration(1, b) } +func BenchmarkKeyRegistration5(b *testing.B) { benchmarkKeyRegistration(5, b) } func BenchmarkKeyRegistration10(b *testing.B) { benchmarkKeyRegistration(10, b) } func BenchmarkKeyRegistration50(b *testing.B) { benchmarkKeyRegistration(50, b) } diff --git a/node/node_test.go b/node/node_test.go index 6a7811e58f..bb0e64754c 100644 --- a/node/node_test.go +++ b/node/node_test.go @@ -548,8 +548,7 @@ func TestAsyncRecord(t *testing.T) { node.RecordAsync(addr, 20000, account.BlockProposal) time.Sleep(5000 * time.Millisecond) - records, err := node.participationRegistry.GetAll() - require.NoError(t, err) + records := node.participationRegistry.GetAll() require.Len(t, records, 1) require.Equal(t, 10000, int(records[0].LastVote)) require.Equal(t, 20000, int(records[0].LastBlockProposal)) From f61267a7b56b2f5d7cd8695952d585442caef0b2 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Tue, 31 Aug 2021 12:43:04 -0400 Subject: [PATCH 09/11] Move the Register early exit up. --- data/account/participationRegistry.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index 5f0e870875..9a22e73d12 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -500,16 +500,16 @@ func (db *participationDB) Register(id ParticipationID, on basics.Round) error { return ErrParticipationIDNotFound } - // round out of valid range. - if on < recordToRegister.FirstValid || on > recordToRegister.LastValid { - return ErrInvalidRegisterRange - } - // No-op If the record is already active if recordActive(recordToRegister, on) { return nil } + // round out of valid range. + if on < recordToRegister.FirstValid || on > recordToRegister.LastValid { + return ErrInvalidRegisterRange + } + updated := make(map[ParticipationID]ParticipationRecord) err := db.store.Wdb.Atomic(func(ctx context.Context, tx *sql.Tx) error { // Disable active key if there is one From 8265faee9383d5096a8801604d64d9a8604f3a4f Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 1 Sep 2021 10:05:36 -0400 Subject: [PATCH 10/11] Downgrade an error to a warning. --- node/node.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/node.go b/node/node.go index dd22e325d9..6a0cd9b581 100644 --- a/node/node.go +++ b/node/node.go @@ -1165,7 +1165,7 @@ func (node *AlgorandFullNode) VotingKeys(votingRound, keysRound basics.Round) [] // This is usually a no-op, but the first time it will update the DB. err := node.participationRegistry.Register(part.ParticipationID(), keysRound) if err != nil { - node.log.Error("Failed to register participation key (%s) with participation registry.", part.ParticipationID()) + node.log.Warn("Failed to register participation key (%s) with participation registry.", part.ParticipationID()) } } // write the warnings per account only if we couldn't find a single valid key for that account. From 5a4b2fbd483a0c64e0bf46247010cf451236f597 Mon Sep 17 00:00:00 2001 From: Will Winder Date: Wed, 8 Sep 2021 11:00:11 -0400 Subject: [PATCH 11/11] Fix Register lock. --- data/account/participationRegistry.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/data/account/participationRegistry.go b/data/account/participationRegistry.go index 9a22e73d12..23a063e6dc 100644 --- a/data/account/participationRegistry.go +++ b/data/account/participationRegistry.go @@ -510,6 +510,8 @@ func (db *participationDB) Register(id ParticipationID, on basics.Round) error { return ErrInvalidRegisterRange } + db.mutex.Lock() + defer db.mutex.Unlock() updated := make(map[ParticipationID]ParticipationRecord) err := db.store.Wdb.Atomic(func(ctx context.Context, tx *sql.Tx) error { // Disable active key if there is one @@ -550,9 +552,6 @@ func (db *participationDB) Register(id ParticipationID, on basics.Round) error { // Update cache if err == nil { - db.mutex.Lock() - defer db.mutex.Unlock() - for id, record := range updated { delete(db.dirty, id) db.cache[id] = record