Add activity tracking to entropy generator #194
Conversation
jinmannwong
commented
Oct 2, 2020
•
jinmannwong
commented
- Track the signature shares received from validators in a sliding window
- Slash when the number of shares in the window is below the allowed threshold
- Do not double slash validators for inactivity in the same period
- Bug fixes from deployment
beacon/entropy_generator.go
Outdated
| } | ||
| evidence.ComplainantSignature = sig | ||
|
|
||
| entropyGenerator.Logger.Debug("Add evidence for inactivity", "height", entropy.Height, "validator", crypto.AddressHash(defAddress)) |
There was a problem hiding this comment.
I would consider making this a log Info. When doing the SDK tests I would have appreciated if the logs said something to the effect of 'jailing/slashing validator for inactivity' when it happened so you aren't wondering why it is suddenly jailed. In my mind there should be logs by default if something unexpected happens
There was a problem hiding this comment.
Sure thing. The slashing module actually does have Info logs when it jails a validator. Not sure why they weren't printing out on the tests.
There was a problem hiding this comment.
Oh - perhaps I missed them. I guess the logs rather than being here could be in the SDK actually, since that would be when action actually happened
| stateDB dbm.DB | ||
| evpool evidencePool | ||
| aeonEntropyParams *types.EntropyParams // Inactivity params for this aeon | ||
| activityTracking map[uint]*list.List // Record of validators |
There was a problem hiding this comment.
Does it need to be a pointer to a list?
beacon/entropy_generator.go
Outdated
| sigShareCount += e.Value.(int) | ||
| } | ||
|
|
||
| threshold := float64(entropyGenerator.aeonEntropyParams.RequiredActivityPercentage*entropyGenerator.aeonEntropyParams.InactivityWindowSize) * 0.01 |
There was a problem hiding this comment.
I might break this up into multiple lines, and also make it a log info for when there is evidence submitted against you - in my mind if something unexpected is happening such as slashing there should be a log so you notice it asap. I found that in the sdk tests it was confusing when you would be suddenly jailed with no indication why, if it had printed 'validator jailed for inactivity' that would have been helpful.
| stateDB dbm.DB | ||
| evpool evidencePool | ||
| aeonEntropyParams *types.EntropyParams // Inactivity params for this aeon | ||
| activityTracking map[uint]*list.List // Record of validators |
There was a problem hiding this comment.
Does this need to be a pointer?
There was a problem hiding this comment.
It is slightly nicer when iterating through the map as you can edit the list directly rather than having to access it by indexing into the map.
| @@ -530,6 +543,9 @@ func (bie *BeaconInactivityEvidence) Verify(chainID string, complainantPubKey cr | |||
| return fmt.Errorf("ComplainantSignature invalid") | |||
| } | |||
|
|
|||
| // Need to verify defendant address in state and also aeon start is correct | |||
| // and evidence height is greater than aeon start | |||
There was a problem hiding this comment.
Evidence needs to be fully verified in Tendermint - coming in #195
