-
Notifications
You must be signed in to change notification settings - Fork 457
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left few comments.
framework/src/engine/consensus/certificate_generation/commit_pool.ts
Outdated
Show resolved
Hide resolved
framework/src/engine/consensus/certificate_generation/commit_pool.ts
Outdated
Show resolved
Hide resolved
framework/src/engine/consensus/certificate_generation/commit_pool.ts
Outdated
Show resolved
Hide resolved
framework/src/engine/consensus/certificate_generation/commit_pool.ts
Outdated
Show resolved
Hide resolved
framework/src/engine/consensus/certificate_generation/commit_pool.ts
Outdated
Show resolved
Hide resolved
cb3c6e2
to
c6074fb
Compare
♻️ Update based on LIP0053 updates 🐛 Filter out bftWeight===0 when saving validatorsPreimage 🐛 Filter out bftWeight===0 when computing validatorsHash 🐛 Filter out bftWeight===0 when selecting and validating aggregateCommit
c6074fb
to
8573a2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two small comments, otherwise looks good. Moreover, don't we need to update tests for verifyValidatorsUpdate
?
Codecov Report
@@ Coverage Diff @@
## release/6.0.0 #8899 +/- ##
=================================================
- Coverage 83.61% 83.60% -0.01%
=================================================
Files 601 601
Lines 22520 22537 +17
Branches 3319 3320 +1
=================================================
+ Hits 18829 18843 +14
- Misses 3691 3694 +3 |
bef3bc7
to
32de658
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one optional comment.
c7acfb8
to
ca17ffe
Compare
a7c472c
to
ae5dbb5
Compare
What was the problem?
This PR resolves #8883 & #8905 & #8849
How was it solved?
getBFTParametersActiveValidators
to provide only active validators from BFT params.verifyValidatorsUpdate
bitmap verification to properly handlebftWeight === 0
inbftWeightUpdate
How was it tested?
Run multiple chains and enable chain connector plugin, change validator set by staking/unstaking and observe CCUs