substrate
substrate copied to clipboard
client/beefy: add some bounds on enqueued votes
When BEEFY voter runs behind rest of voters it will enqueue gossiped votes and process them as it catches up.
We should introduce some bounds on this queue to not grow forever if voter is stuck - we should instead drop votes once bounds are hit.
@acatangiu I will like to work on this
am working on it
am working on it
Oh... I already started @MrishoLukamba
Okey continue
So, I am currently working on this, @acatangiu
I am imploring the use of a BoundedBTreeMap for the buffer to hold votes
BoundedBTreeMap<NumberFor<B>, Vec<VoteMessage<NumberFor<B>, AuthorityId, Signature>>, ConstU32<U>> where U is a const type parameter.
There is a to_process_for_bound which is inside try_pending_justif_and_votes which i will need to split due to the new structure that the pending_votes is attaining with the BoundedBTreeMap.
is it ok to split the function? or should I go ahead to make pending_votes and also pending_justifications a BoundedBTreeMap structure?
I am trying to ask so as to confirm if i am on the right track. @acatangiu
Yes, the approach is good and you should put bounds on both pending_votes and pending_justifications.
You should also bound inner vec in pending_votes, use BoundedVec or some simple custom vec wrapper for BoundedBTreeMap<NumberFor<B>, BoundedVec<_, _>>.
Bounds sizes should be smth in the order of a few MBs for each map - just define some constants, document them and we can fine tune the value as a final detail.
For the inner votes Vec it would be great to have some way to configure bound based on authority_set size (basically make it runtime configurable instead of hardcoded const).
When bounds are reached:
- votes can be dropped with a
warn!log, - justifications can be dropped with a
error!log (in the future we might even decide to stop the voter with "fatal" error for dropping justifications, but for now let's start with logging errors).
You should also bound inner vec in
pending_votes, useBoundedVecor some simple custom vec wrapper forBoundedBTreeMap<NumberFor<B>, BoundedVec<_, _>>.Bounds sizes should be smth in the order of a few MBs for each map - just define some constants, document them and we can fine tune the value as a final detail.
For the inner votes
Vecit would be great to have some way to configure bound based on authority_set size (basically make it runtime configurable instead of hardcoded const).When bounds are reached:
- votes can be dropped with a
warn!log,- justifications can be dropped with a
error!log (in the future we might even decide to stop the voter with "fatal" error for dropping justifications, but for now let's start with logging errors).
Alright @acatangiu I Will open a PR soon enough