PKI TODO comments + fix import order. (#3167)

Author: winder

data/account/participationRegistry.go

@@ -813,6 +813,11 @@ func recordActive(record ParticipationRecord, on basics.Round) bool {
 	return record.EffectiveLast != 0 && record.EffectiveFirst <= on && on <= record.EffectiveLast
 }
 
+// PKI TODO: Register needs a bit more work to make sure EffectiveFirst and
+//           EffectiveLast are set at the right time. Specifically, the node
+//           doesn't call Register until the key becomes active and is about
+//           to be used, so effective first/last is updated just-in-time. It
+//           would be better to update them when the KeyRegistration occurs.
 func (db *participationDB) Register(id ParticipationID, on basics.Round) error {
 	// Lookup recordToRegister for first/last valid and account.
 	recordToRegister := db.Get(id)
@@ -843,8 +848,7 @@ func (db *participationDB) Register(id ParticipationID, on basics.Round) error {
 
 	// Disable active key if there is one
 	for _, record := range toUpdate {
-		// TODO: this should probably be "on - 1"
-		record.EffectiveLast = on
+		record.EffectiveLast = on - 1
 		updated[record.ParticipationID] = updatingParticipationRecord{
 			record.Duplicate(),
 			false,

data/account/participationRegistry_test.go

@@ -211,7 +211,7 @@ func TestParticipation_Register(t *testing.T) {
 	// Register second key.
 	err = registry.Register(p2.ID(), 2500000)
 	a.NoError(err)
-	verifyEffectiveRound(p.ID(), 500000, 2500000)
+	verifyEffectiveRound(p.ID(), 500000, 2499999)
 	verifyEffectiveRound(p2.ID(), 2500000, int(p2.LastValid))
 }
 

data/accountManager.go

@@ -67,7 +67,7 @@ func (manager *AccountManager) Keys(rnd basics.Round) (out []account.Participati
 	}
 	return out
 
-	// TODO: source keys from the registry.
+	// PKI TODO: source keys from the registry.
 	// This kinda works, but voting keys are not updated.
 	/*
 		for _, record := range manager.registry.GetAll() {
@@ -209,7 +209,7 @@ func (manager *AccountManager) DeleteOldKeys(latestHdr bookkeeping.BlockHeader,
 		}
 	}
 
-	// TODO: This needs to update the partkeys also, see the 'DeleteOldKeys' function above, it's part
+	// PKI TODO: This needs to update the partkeys also, see the 'DeleteOldKeys' function above, it's part
 	//       is part of PersistedParticipation, but just calls 'part.Voting.DeleteBeforeFineGrained'
 	// Delete expired records from participation registry.
 	if err := manager.registry.DeleteExpired(latestHdr.Round); err != nil {

node/node.go

@@ -415,7 +415,7 @@ func (node *AlgorandFullNode) Start() {
 func (node *AlgorandFullNode) startMonitoringRoutines() {
 	node.monitoringRoutinesWaitGroup.Add(3)
 
-	// TODO: Remove this with #2596
+	// PKI TODO: Remove this with #2596
 	// Periodically check for new participation keys
 	go node.checkForParticipationKeys()
 

util/db/initialize_test.go

@@ -20,9 +20,10 @@ import (
 	"context"
 	"database/sql"
 	"errors"
-	"github.com/algorand/go-algorand/test/partitiontest"
 	"testing"
 
+	"github.com/algorand/go-algorand/test/partitiontest"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )