chore: Refactor CommitPubRand
#233
Conversation
gitferry
force-pushed
the
gai/add-randomness-ahead-height
branch
from
4ca005d to
dc93487
Compare
| @@ -316,9 +373,17 @@ func (fp *FinalityProviderInstance) hasVotingPower(b *types.BlockInfo) (bool, er | |||
| zap.Uint64("block_height", b.Height), | |||
| ) | |||
|
|
|||
| if fp.GetStatus() == proto.FinalityProviderStatus_ACTIVE { | |||
| fp.MustSetStatus(proto.FinalityProviderStatus_INACTIVE) | |||
There was a problem hiding this comment.
Nit: not a huge fan of this, it's a bit hidden away that this function updates state. I suggest making it return the voting power or something like (bool, uint64, err), and then the function caller can decide what to do with it.
Alternatively, add comment saying that it also updates local status
There was a problem hiding this comment.
Good point. Did some refactoring in 757156e
| @@ -173,41 +171,42 @@ func (fp *FinalityProviderInstance) finalitySigSubmissionLoop() { | |||
| defer fp.wg.Done() | |||
|
|
|||
| for { | |||
| // start submission in the first iteration | |||
| pollerBlocks := fp.getAllBlocksFromChan() | |||
There was a problem hiding this comment.
nice, I bet we do this a lot throughout the codebases, that we wait before the first iteration
There was a problem hiding this comment.
I just realized that we can't do it for every loop for cases where we have continue upon error. If we do, then we the loop will go forever. Need to figure out other ways for it
gitferry
force-pushed
the
gai/add-randomness-ahead-height
branch
2 times, most recently
from
9c904b3 to
5ec5a77
Compare
gitferry
force-pushed
the
gai/add-randomness-ahead-height
branch
from
5ec5a77 to
edb1eb0
Compare
Closes #225. The major goal of this pr is to refactor
CommitPubRandbut turned out included many small refactorings along the way (sorry for the mixup). Notable changes are:retryCommitPubRandUntilBlockFinalizedintoShouldCommitRandomnessandCommitPubRand. The former checks whether a new commit should be made based on the last committed height and the tip height. Iftrue, it also returns the start height of the commit. The latter makes a new commit based on the start height returned by the former.config.MinRandHeightGaptoconfig.TimestampingDelayBlockswhich is an estimated value of the latency between a commit is submitted and it is timestamped by the btc-timestamping protocolProcessBlocksToVoteReviewers, please take a careful look at
fp_instance.go, especially howShouldCommitRandomnessdetermines the start height with consideration of btc-timestamping delay