Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rosetta Implementation Cleanup FIX (Stage 3 of Node API Overhaul) #3402

Merged

Conversation

Daniel-VDM
Copy link
Contributor

@Daniel-VDM Daniel-VDM commented Oct 23, 2020

Stage 3 Cleanup FIX of Node API Overhaul

This is related to #3390

In the clean-up PR, I attempted to fix the pre-staking block 'off-by-one' errors that I got in my testing using the rosetta-cli. However, in further fuzzing-esk testing (different committee sizes, different transactions going off in the pre-staking era, etc...) I found that in some (rare) cases, there would still be an off-by-one error.

I've now changed the pre-staking block reward calculations to call the exact same functions as in chain.AccumulateRewardsAndCountSigs, even when fetching the block signers. I've tested this fix against said test and it appears to work.

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
* Add GetPreStakingBlockRewards with cache

Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM added the rpc RPC or API label Oct 23, 2020
@Daniel-VDM Daniel-VDM requested review from rlan35 and LeoHChen October 23, 2020 13:48
@Daniel-VDM Daniel-VDM self-assigned this Oct 23, 2020
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM force-pushed the rosetta-pre-staking-reward-fix branch from c8977d7 to 5ee1e79 Compare October 23, 2020 13:57
Signed-off-by: Daniel Van Der Maden <dvandermaden0@berkeley.edu>
@Daniel-VDM Daniel-VDM merged commit 046e87e into harmony-one:main Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rpc RPC or API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants