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

start_from_genesis.py doomslug_off is flaky #3138

Closed
SkidanovAlex opened this issue Aug 11, 2020 · 2 comments · Fixed by #3321
Closed

start_from_genesis.py doomslug_off is flaky #3138

SkidanovAlex opened this issue Aug 11, 2020 · 2 comments · Fixed by #3321
Assignees
Labels
A-chain Area: Chain, client & related

Comments

@SkidanovAlex
Copy link
Collaborator

E.g.

http://52.149.162.182:3000/#/test/7284
http://52.149.162.182:3000/#/test/7644

Traceback (most recent call last):
  File "tests/adversarial/start_from_genesis.py", line 86, in <module>
    assert saved_blocks_2['result'] < saved_blocks['result'] + 10
AssertionError

@mikhailOK
Copy link
Contributor

What the test is doing:

  1. start node0 and node1, wait for 30 blocks
  2. stop node0 and node1, clear data of node1
  3. restart node1 and set adv_disable_header_sync
  4. restart node0
  5. Use adv_produce_blocks on node1, producing 10 blocks from current head
  6. verify that node0 did not save those blocks

The test expects that node1 does not not sync, and produces 10 blocks from genesis to height ~20, so node0 should not save them. But node1 can still sync: some time after starting, node0 produces a new block and sends it to node1, and node1 request the chain of parents through orphan logic in receive_block.

So, if node1 syncs between steps 4 and 5, the test fails, but when it happens after step 5 test passes.

Possible ways to fix the test:

  • do adv_produce_blocks before restarting test0 - does the test need this order?
  • disable requesting orphan parents with adv_disable_header_sync (or make a new adversarial switch?)

@SkidanovAlex
Copy link
Collaborator Author

The first way should be OK, I don't think there's any reason for the current order of those two operations

mikhailOK added a commit that referenced this issue Sep 14, 2020
Fixes #3138
Make the test do adv_produce_blocks when the node is disconnected,
so it doesn't accidentally sync and produce blocks on top of main chain.

Also remove disabling header sync, because it disables sync in both
directions and prevents the correct node from getting malicious headers.

Test plan
---------
Run start_from_genesis.py in all modes many times
mikhailOK added a commit that referenced this issue Sep 14, 2020
Fixes #3138
Make the test do adv_produce_blocks when the node is disconnected,
so it doesn't accidentally sync and produce blocks on top of main chain.

Also remove disabling header sync, because it disables sync in both
directions and prevents the correct node from getting malicious headers.

Test plan
---------
Run start_from_genesis.py in all modes many times
mikhailOK added a commit that referenced this issue Sep 14, 2020
Fixes #3138
Make the test do adv_produce_blocks when the node is disconnected,
so it doesn't accidentally sync and produce blocks on top of main chain.

Also remove disabling header sync, because it disables sync in both
directions and prevents the correct node from getting malicious headers.

Test plan
---------
Run start_from_genesis.py in all modes many times
mikhailOK added a commit that referenced this issue Sep 15, 2020
Fixes #3138
Make the test do adv_produce_blocks when the node is disconnected,
so it doesn't accidentally sync and produce blocks on top of main chain.

Also remove disabling header sync, because it disables sync in both
directions and prevents the correct node from getting malicious headers.

Test plan
---------
Run start_from_genesis.py in all modes many times
chefsale pushed a commit that referenced this issue Sep 21, 2020
Fixes #3138
Make the test do adv_produce_blocks when the node is disconnected,
so it doesn't accidentally sync and produce blocks on top of main chain.

Also remove disabling header sync, because it disables sync in both
directions and prevents the correct node from getting malicious headers.

Test plan
---------
Run start_from_genesis.py in all modes many times
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-chain Area: Chain, client & related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants