[Merged by Bors] - Self-healing #2400

lrettig · 2021-04-28T19:06:09Z

Motivation

See spacemeshos/SMIPS#46

Requires #2394, #2393, #2357
Closes #2203
Closes #2687

Changes

adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector)
Hare explicitly reports failed CPs to the mesh, with the results stored in memory

Test Plan

TBD

TODO

DevOps Notes

This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
This PR does not affect public APIs
This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
This PR does not make changes to log messages (which monitoring infrastructure may rely on)

Bring context down into verifying tortoise

Hare reports to mesh when it explicitly fails for a layer. This allows tortoise to add votes against all blocks in such a layer. Also adds some more minor cleanup.

Increase default hdist Invalidate layers with no input vector older than zdist Also clean up some interfaces

Comments

- Set local opinion as opposed to all blocks in a layer if Hare failed for that layer. - Correctly process exception votes when processing blocks (rather than summing base block vote + exception vote); handle abstain votes here - Correctly interpret the lack of an explicit vote in support of a block in an older late as a vote against that block when summing global opinion Lots of minor cleanup and improvements of comments, logs, variable names Change some panics to errors

tortoise/verifying_tortoise.go

Per spacemeshos/research#33 we want to look back to the start of the sliding window (currently defined as hdist) when adding exceptions to a candidate base block

Make comments clearer, some minor cleanup

- Mostly finished self-healing core logic in tortoise - Remove lots of unused code from block builder - Adds weak coin stub to hare while waiting for #2393 to be merged

Change direction of data flow, hare -> mesh -> tortoise

Improve some log output

Remove unused variables, clean up method signatures

Regarding block and ATX weights, per Tal

dshulyak · 2021-08-26T03:25:18Z

bors merge

## Motivation See spacemeshos/SMIPS#46 Requires #2394, #2393, #2357 Closes #2203 Closes #2687 ## Changes - adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector) - Hare explicitly reports failed CPs to the mesh, with the results stored in memory ## Test Plan TBD ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Finish rescoring block goodness after healing - [x] Rescoring block unit test - [x] Block voting weight unit test - [x] Make sure healing -> verifying tortoise handoff is working - [x] App test (for decay in active set size) - [x] Split-then-rejoin test (two way split, three way split w/o majority fork) - [x] Late blocks unit test(s) - [x] Correctly weight votes from blocks with late atxs/those with a bad beacon value (will follow up in separate issue: #2540) - [x] Multi tortoise unit tests ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [ ] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2021-08-26T03:36:06Z

Build failed:

ci-stage2

dshulyak · 2021-08-26T03:48:15Z

weird. different unit test is failing every time, it didn't even come to latenodes in last 4 tries

bors merge

## Motivation See spacemeshos/SMIPS#46 Requires #2394, #2393, #2357 Closes #2203 Closes #2687 ## Changes - adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector) - Hare explicitly reports failed CPs to the mesh, with the results stored in memory ## Test Plan TBD ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Finish rescoring block goodness after healing - [x] Rescoring block unit test - [x] Block voting weight unit test - [x] Make sure healing -> verifying tortoise handoff is working - [x] App test (for decay in active set size) - [x] Split-then-rejoin test (two way split, three way split w/o majority fork) - [x] Late blocks unit test(s) - [x] Correctly weight votes from blocks with late atxs/those with a bad beacon value (will follow up in separate issue: #2540) - [x] Multi tortoise unit tests ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [ ] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2021-08-26T04:04:34Z

Build failed:

ci-stage2

dshulyak · 2021-08-26T04:06:08Z

bors merge

## Motivation See spacemeshos/SMIPS#46 Requires #2394, #2393, #2357 Closes #2203 Closes #2687 ## Changes - adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector) - Hare explicitly reports failed CPs to the mesh, with the results stored in memory ## Test Plan TBD ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Finish rescoring block goodness after healing - [x] Rescoring block unit test - [x] Block voting weight unit test - [x] Make sure healing -> verifying tortoise handoff is working - [x] App test (for decay in active set size) - [x] Split-then-rejoin test (two way split, three way split w/o majority fork) - [x] Late blocks unit test(s) - [x] Correctly weight votes from blocks with late atxs/those with a bad beacon value (will follow up in separate issue: #2540) - [x] Multi tortoise unit tests ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [ ] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2021-08-26T04:16:51Z

Build failed:

ci-stage2

dshulyak · 2021-08-26T04:38:01Z

i don't understand why unit tests are so flaky, there are now 3 failures in a single run:

TestMesh_AddBlockWithTxs_PushTransactions_UpdateUnappliedTxs
TestMesh_updateStateWithLayer
TestFindNodeProtocol_FindNode_Concurrency
i just restarted them on develop in my branch and they passed from the first try [Merged by Bors] - tortoise: refactor trtl.Persist to use its own store and do the task more efficiently #2683

bors merge

## Motivation See spacemeshos/SMIPS#46 Requires #2394, #2393, #2357 Closes #2203 Closes #2687 ## Changes - adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector) - Hare explicitly reports failed CPs to the mesh, with the results stored in memory ## Test Plan TBD ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Finish rescoring block goodness after healing - [x] Rescoring block unit test - [x] Block voting weight unit test - [x] Make sure healing -> verifying tortoise handoff is working - [x] App test (for decay in active set size) - [x] Split-then-rejoin test (two way split, three way split w/o majority fork) - [x] Late blocks unit test(s) - [x] Correctly weight votes from blocks with late atxs/those with a bad beacon value (will follow up in separate issue: #2540) - [x] Multi tortoise unit tests ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [ ] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2021-08-26T04:47:39Z

Build failed:

ci-stage2

dshulyak · 2021-08-26T06:24:57Z

bors merge

## Motivation See spacemeshos/SMIPS#46 Requires #2394, #2393, #2357 Closes #2203 Closes #2687 ## Changes - adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector) - Hare explicitly reports failed CPs to the mesh, with the results stored in memory ## Test Plan TBD ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Finish rescoring block goodness after healing - [x] Rescoring block unit test - [x] Block voting weight unit test - [x] Make sure healing -> verifying tortoise handoff is working - [x] App test (for decay in active set size) - [x] Split-then-rejoin test (two way split, three way split w/o majority fork) - [x] Late blocks unit test(s) - [x] Correctly weight votes from blocks with late atxs/those with a bad beacon value (will follow up in separate issue: #2540) - [x] Multi tortoise unit tests ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [ ] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2021-08-26T06:41:13Z

Build failed:

ci-stage2

dshulyak · 2021-08-26T06:44:29Z

There must be something in this branch that makes those tests crash consistently on CI

Layer arithmetic

More layer arithmetic fixes

lrettig · 2021-08-26T20:14:11Z

bors merge

## Motivation See spacemeshos/SMIPS#46 Requires #2394, #2393, #2357 Closes #2203 Closes #2687 ## Changes - adds zdist param: whereas hdist is the "Hare lookback" distance (the number of layers for which we consider hare results/local input vector rather than global opinion), zdist is the "Hare result wait" distance (the number of layers we're willing to wait for Hare to finish before reverting to invalidating layers with no input vector) - Hare explicitly reports failed CPs to the mesh, with the results stored in memory ## Test Plan TBD ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Finish rescoring block goodness after healing - [x] Rescoring block unit test - [x] Block voting weight unit test - [x] Make sure healing -> verifying tortoise handoff is working - [x] App test (for decay in active set size) - [x] Split-then-rejoin test (two way split, three way split w/o majority fork) - [x] Late blocks unit test(s) - [x] Correctly weight votes from blocks with late atxs/those with a bad beacon value (will follow up in separate issue: #2540) - [x] Multi tortoise unit tests ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [ ] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2021-08-26T21:09:19Z

Pull request successfully merged into develop.

Build succeeded:

ci-stage2

lrettig · 2021-08-26T21:19:38Z

i don't understand why unit tests are so flaky, there are now 3 failures in a single run:

TestMesh_AddBlockWithTxs_PushTransactions_UpdateUnappliedTxs

TestMesh_updateStateWithLayer

TestFindNodeProtocol_FindNode_Concurrency
i just restarted them on develop in my branch and they passed from the first try Refactor trtl.Persist to use its own store and do the task more efficiently #2683

These failures were due to a bugfix I made in the mesh code, just needed to update those tests - TestFindNodeProtocol_FindNode_Concurrency is flaky but the others were not a flakiness issue

countvonzero · 2021-09-03T00:28:58Z

mesh/reward_test.go

 	// test states are the same when one input is from tortoise and the other from hare
-	assert.Equal(t, s.Txs, s2.Txs)
+	require.NotEqual(t, s.Txs, s2.Txs)


@lrettig
is this correct? if we want the same result, it should be equal right?

the comment is a little misleading (see identical comment a few lines down), but the change is intentional and this should be correct. all layers except the latest layer have been applied here, so the state should not be the same. then the latest layer is applied, and then the state should be the same. that's what this is testing.

countvonzero · 2021-09-03T00:37:02Z

mesh/reward_test.go

+	mesh2.ValidateLayer(context.TODO(), l)
+
+	// test states are the same when one input is from tortoise and the other from hare
+	require.Equal(t, s.Txs, s2.Txs)


why not do for i := 0; i < numOfLayers; i++ in line 228 and require.Equal(t, s.Txs, s2.Txs) directly?
why care about this one last layer difference?

the test is more thorough if it isolates the final layer application: the states are not equal before the final layer is applied, and they are equal after.

lrettig added 7 commits April 27, 2021 16:48

Context and logging

eaec25f

Bring context down into verifying tortoise

Gofmt

95681ee

More context and logging

40dc7b3

Cleanup: logging and comments

f838209

Add support for explicitly invalid layers

927df39

Hare reports to mesh when it explicitly fails for a layer. This allows tortoise to add votes against all blocks in such a layer. Also adds some more minor cleanup.

Add zdist

8fac2c6

Increase default hdist Invalidate layers with no input vector older than zdist Also clean up some interfaces

Gofmt

9f327cf

lrettig added the self-healing label Apr 28, 2021

lrettig self-assigned this Apr 28, 2021

lrettig added 3 commits April 30, 2021 12:47

Merge branch 'develop' into healing

293c114

Minor cleanup

573e17f

Comments

lrettig commented Apr 30, 2021

View reviewed changes

lrettig mentioned this pull request May 1, 2021

explicit votes distance #1430

Closed

lrettig added 5 commits May 1, 2021 15:16

Add exceptions back to hDist

9c59960

Per spacemeshos/research#33 we want to look back to the start of the sliding window (currently defined as hdist) when adding exceptions to a candidate base block

Add tortoise confidence param

4d79394

Make comments clearer, some minor cleanup

Gofmt

664a253

Add initial criteria to trigger self-healing

de3d96b

Clarified point with Tal, remove TODO

0bdf5e5

This was referenced May 1, 2021

Weak Coin for self-healing spacemeshos/pm#28

Closed

Self Healing spacemeshos/pm#27

Closed

lrettig added 9 commits May 1, 2021 20:04

Self-healing core logic, add weak coin stub

e057d6b

- Mostly finished self-healing core logic in tortoise - Remove lots of unused code from block builder - Adds weak coin stub to hare while waiting for #2393 to be merged

Cleanup weak coin stub

5ccff1c

Change direction of data flow, hare -> mesh -> tortoise

Linting

fb22771

Gofmt

0ccfe77

Finish core healing logic

95b8049

Improve some log output

Logic cleanup

c548378

Remove unused variables, clean up method signatures

Add explanatory comment

e629c64

Regarding block and ATX weights, per Tal

Merge branch 'develop' into healing

9701f5a

Add WindowSize

3694e1d

lrettig added 3 commits August 26, 2021 15:41

Fix mesh test

8402d81

Layer arithmetic

Fix test

66a8c21

More layer arithmetic fixes

Merge branch 'develop' into healing

c6f22d1

bors bot changed the title ~~Self-healing~~ [Merged by Bors] - Self-healing Aug 26, 2021

bors bot closed this Aug 26, 2021

bors bot deleted the healing branch August 26, 2021 21:09

This was referenced Aug 26, 2021

result received after state has advanced warning #2685

Closed

Verified layer stuck (Release v0.2.0-rc4) #2689

Closed

countvonzero reviewed Sep 3, 2021

View reviewed changes

[Merged by Bors] - Self-healing #2400

[Merged by Bors] - Self-healing #2400

lrettig commented Apr 28, 2021 •

edited

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

lrettig commented Aug 26, 2021

bors bot commented Aug 26, 2021

lrettig commented Aug 26, 2021

countvonzero Sep 3, 2021

lrettig Sep 6, 2021

countvonzero Sep 3, 2021

lrettig Sep 6, 2021

[Merged by Bors] - Self-healing #2400

[Merged by Bors] - Self-healing #2400

Conversation

lrettig commented Apr 28, 2021 • edited

Motivation

Changes

Test Plan

TODO

DevOps Notes

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

bors bot commented Aug 26, 2021

dshulyak commented Aug 26, 2021

lrettig commented Aug 26, 2021

bors bot commented Aug 26, 2021

lrettig commented Aug 26, 2021

countvonzero Sep 3, 2021

Choose a reason for hiding this comment

lrettig Sep 6, 2021

Choose a reason for hiding this comment

countvonzero Sep 3, 2021

Choose a reason for hiding this comment

lrettig Sep 6, 2021

Choose a reason for hiding this comment

lrettig commented Apr 28, 2021 •

edited