Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Detect when valid partkeys represent an offline or closed account #4030

Merged
merged 10 commits into from
Jul 19, 2022
Merged
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 @@ -1314,6 +1321,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 @@ -1327,12 +1351,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 @@ -1341,6 +1366,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 @@ -1363,14 +1390,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 @@ -553,3 +553,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) {
AlgoStephenAkiki marked this conversation as resolved.
Show resolved Hide resolved
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))
})
}
}