Skip to content

Commit

Permalink
Enhancement: Detect when valid partkeys represent an offline or close…
Browse files Browse the repository at this point in the history
…d account (#4030)

Puts in booleans to deduct account status from online account data and
uses that to upgrade log messages to info from warnings if necessary
  • Loading branch information
AlgoStephenAkiki authored Jul 19, 2022
1 parent a7be175 commit 5519656
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 8 deletions.
51 changes: 43 additions & 8 deletions node/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ const (
participationRegistryFlushMaxWaitDuration = 30 * time.Second
)

const (
bitMismatchingVotingKey = 1 << iota
bitMismatchingSelectionKey
bitAccountOffline
bitAccountIsClosed
)

// StatusReport represents the current basic status of the node
type StatusReport struct {
LastRound basics.Round
Expand Down Expand Up @@ -1258,6 +1265,23 @@ func (node *AlgorandFullNode) AssembleBlock(round basics.Round) (agreement.Valid
return validatedBlock{vb: lvb}, nil
}

// getOfflineClosedStatus will return an int with the appropriate bit(s) set if it is offline and/or online
func getOfflineClosedStatus(acctData basics.OnlineAccountData) int {
rval := 0
isOffline := acctData.VoteFirstValid == 0 && acctData.VoteLastValid == 0

if isOffline {
rval = rval | bitAccountOffline
}

isClosed := isOffline && acctData.MicroAlgosWithRewards.Raw == 0
if isClosed {
rval = rval | bitAccountIsClosed
}

return rval
}

// VotingKeys implements the key manager's VotingKeys method, and provides additional validation with the ledger.
// that allows us to load multiple overlapping keys for the same account, and filter these per-round basis.
func (node *AlgorandFullNode) VotingKeys(votingRound, keysRound basics.Round) []account.ParticipationRecordForRound {
Expand All @@ -1271,12 +1295,13 @@ func (node *AlgorandFullNode) VotingKeys(votingRound, keysRound basics.Round) []
accountsData := make(map[basics.Address]basics.OnlineAccountData, len(parts))
matchingAccountsKeys := make(map[basics.Address]bool)
mismatchingAccountsKeys := make(map[basics.Address]int)
const bitMismatchingVotingKey = 1
const bitMismatchingSelectionKey = 2

for _, p := range parts {
acctData, hasAccountData := accountsData[p.Account]
if !hasAccountData {
var err error
// LookupAgreement is used to look at the past ~320 rounds of account state
// It provides a fast lookup method for online account information
acctData, err = node.ledger.LookupAgreement(keysRound, p.Account)
if err != nil {
node.log.Warnf("node.VotingKeys: Account %v not participating: cannot locate account for round %d : %v", p.Account, keysRound, err)
Expand All @@ -1285,6 +1310,8 @@ func (node *AlgorandFullNode) VotingKeys(votingRound, keysRound basics.Round) []
accountsData[p.Account] = acctData
}

mismatchingAccountsKeys[p.Account] = mismatchingAccountsKeys[p.Account] | getOfflineClosedStatus(acctData)

if acctData.VoteID != p.Voting.OneTimeSignatureVerifier {
mismatchingAccountsKeys[p.Account] = mismatchingAccountsKeys[p.Account] | bitMismatchingVotingKey
continue
Expand All @@ -1307,14 +1334,22 @@ func (node *AlgorandFullNode) VotingKeys(votingRound, keysRound basics.Round) []
if matchingAccountsKeys[mismatchingAddr] {
continue
}
if warningFlags&bitMismatchingVotingKey == bitMismatchingVotingKey {
node.log.Warnf("node.VotingKeys: Account %v not participating on round %d: on chain voting key differ from participation voting key for round %d", mismatchingAddr, votingRound, keysRound)
continue
}
if warningFlags&bitMismatchingSelectionKey == bitMismatchingSelectionKey {
node.log.Warnf("node.VotingKeys: Account %v not participating on round %d: on chain selection key differ from participation selection key for round %d", mismatchingAddr, votingRound, keysRound)
if warningFlags&bitMismatchingVotingKey != 0 || warningFlags&bitMismatchingSelectionKey != 0 {
// If we are closed, upgrade this to info so we don't spam telemetry reporting
if warningFlags&bitAccountIsClosed != 0 {
node.log.Infof("node.VotingKeys: Address: %v - Account was closed but still has a participation key active.", mismatchingAddr)
} else if warningFlags&bitAccountOffline != 0 {
// If account is offline, then warn that no registration transaction has been issued or that previous registration transaction is expired.
node.log.Warnf("node.VotingKeys: Address: %v - Account is offline. No registration transaction has been issued or a previous registration transaction has expired", mismatchingAddr)
} else {
// If the account isn't closed/offline and has a valid participation key, then this key may have been generated
// on a different node.
node.log.Warnf("node.VotingKeys: Account %v not participating on round %d: on chain voting key differ from participation voting key for round %d. Consider regenerating the participation key for this node.", mismatchingAddr, votingRound, keysRound)
}

continue
}

}
return participations
}
Expand Down
21 changes: 21 additions & 0 deletions node/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,24 @@ func TestAsyncRecord(t *testing.T) {
require.Equal(t, 10000, int(records[0].LastVote))
require.Equal(t, 20000, int(records[0].LastBlockProposal))
}

// TestOfflineOnlineClosedBitStatus a test that validates that the correct bits are being set
func TestOfflineOnlineClosedBitStatus(t *testing.T) {
partitiontest.PartitionTest(t)

tests := []struct {
name string
acctData basics.OnlineAccountData
expectedInt int
}{
{"online 1", basics.OnlineAccountData{VoteFirstValid: 1, VoteLastValid: 100, MicroAlgosWithRewards: basics.MicroAlgos{Raw: 0}}, 0},
{"online 2", basics.OnlineAccountData{VoteFirstValid: 1, VoteLastValid: 100, MicroAlgosWithRewards: basics.MicroAlgos{Raw: 1}}, 0},
{"offline & not closed", basics.OnlineAccountData{VoteFirstValid: 0, VoteLastValid: 0, MicroAlgosWithRewards: basics.MicroAlgos{Raw: 1}}, 0 | bitAccountOffline},
{"offline & closed", basics.OnlineAccountData{VoteFirstValid: 0, VoteLastValid: 0, MicroAlgosWithRewards: basics.MicroAlgos{Raw: 0}}, 0 | bitAccountOffline | bitAccountIsClosed},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
require.Equal(t, test.expectedInt, getOfflineClosedStatus(test.acctData))
})
}
}

0 comments on commit 5519656

Please sign in to comment.