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.

Sign up for GitHub

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

Jump to bottom

Fix unit test rpc.account_representative_set_epoch_2 #3774

Merged
merged 1 commit into from
Apr 21, 2022
Merged

Fix unit test rpc.account_representative_set_epoch_2 #3774

merged 1 commit into from
Apr 21, 2022

Conversation

dsiganos
Copy link
Contributor

@dsiganos dsiganos commented Apr 1, 2022

This test has 2 problems:

  • it has a race condition between upgrading to epoch2 and sending the rpc
    command to change the rep
  • the rpc command is given a work value that is incorrect because the
    wrong root is used to calculate it, since the work value is meant to be
    insufficient, it works most of the time anyway, but there are times when
    the value becomes sufficient and then the test fails

Solutions

  • wait until the epoch upgrades blocks are cemented before sending the rpc
  • use the correct root (hash of epoch2 block) to calculate the work field

resolves #3770

@dsiganos
Fix unit test rpc.account_representative_set_epoch_2
9aa84d6 
This test has 2 problems:
* it has a race condition between upgrading to epoch2 and sending the rpc
  command to change the rep
* the rpc command is given a work value that is incorrect because the
  wrong root is used to calculate it, since the work value is meant to be
  insufficient, it works most of the time anyway, but there are times when
  the value becomes sufficient and then the test fails

Solutions
* wait until the epoch upgrades blocks are cemented before sending the rpc
* use the correct root (hash of epoch2 block) to calculate the work field

resolves nanocurrency#3770
@dsiganos dsiganos requested review from clemahieu, theohax and thsfs April 1, 2022 22:07
@zhyatt zhyatt added the unit test Related to a new, changed or fixed unit test label Apr 1, 2022
@zhyatt zhyatt added this to the V24.0 milestone Apr 1, 2022
@dsiganos dsiganos merged commit bf8ad11 into nanocurrency:develop Apr 21, 2022
@dsiganos dsiganos deleted the fix_test_rpc_account_representative_set_epoch_2_issue3770 branch April 21, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clemahieu clemahieu clemahieu approved these changes

@theohax theohax Awaiting requested review from theohax

@thsfs thsfs Awaiting requested review from thsfs

Assignees
No one assigned
Labels
unit test Related to a new, changed or fixed unit test
Projects
None yet
Milestone
V24.0
Development

Successfully merging this pull request may close these issues.

unit test rpc.account_representative_set_epoch_2 failed in CI
3 participants
@dsiganos @clemahieu @zhyatt