analyzer: damask: Warn about incompatibility with slow-sync #565
Conversation
analyzer/consensus/consensus.go
Outdated
| @@ -669,6 +669,13 @@ func (m *processor) queueEscrows(batch *storage.QueryBatch, data *stakingData) e | |||
| escrower := e.Escrow.String() | |||
| amount := e.Amount.String() | |||
| newShares := e.NewShares.String() | |||
| if e.NewShares.IsZero() { | |||
There was a problem hiding this comment.
we routinely have a lot of "zero" new shares add ecrow events because that's how staking rewards appear
There was a problem hiding this comment.
Huh well spotted. Thank you. I went ahead and made the NewShares field optional in our internal structure; now we can tell whether the node provided the field or not, and only warn for nodes with Cobalt-like API (when the field was actively missing).
There was a problem hiding this comment.
can you confirm if staking rewards explicitly have a zero? there's too many layers of go-esque omitempty and 'use nil instead of empty slice' stuff
There was a problem hiding this comment.
Yes, they do. Or, to be exact: as you predicted, the old warning that triggered whenever e.NewShares.IsZero() produced a lot of noise. I assume those 0-shares events came from staking rewards. Regardless, they exist, and the new tweak on the code does not produce those spurious warnings.
mitjat
force-pushed
the
mitjat/warn-absent-new-shares
branch
from
5851ff5 to
127bba5
Compare
mitjat
force-pushed
the
mitjat/warn-absent-new-shares
branch
from
127bba5 to
94940f3
Compare
mitjat
force-pushed
the
mitjat/warn-absent-new-shares
branch
from
94940f3 to
698afd9
Compare
Certain old heights cannot be indexed using slow-sync without causing incorrectly dead-reckoned values in the DB.
A more thorough solution would be to separately query the node for the absolute number of shares whenever we encounter an
AddEscrowwithoutnew_shares. But we're unlikely to really need to support that mode of operation, so let's do the simple thing (= this PR).