Implement new mechanism for updating UTXO Diffs #1671

svarogg · 2021-04-11T11:26:36Z

This PR implements a mechanism described in the following document: https://github.com/kaspanet/docs/blob/main/Specs/Updates%20to%20UTXO%20Diffs.md

Among other things, this enables a re-org of finalityWindow blocks, without a crashing node or a node stuck forever.
Nevertheless, it still takes around 2 hours to re-org a whole finalityWindow.

This fixes #1516

…in restoreSingleBlockStatus

… testConsensus

…electedTip

…lock

…head-of-time

This reverts commit c0af9dc.

This reverts commit 4fcca1b.

codecov · 2021-04-11T11:33:27Z

Codecov Report

Merging #1671 (d4e4516) into v0.10.0-dev (28bfc0f) will increase coverage by 0.17%.
The diff coverage is 66.29%.

@@               Coverage Diff               @@
##           v0.10.0-dev    #1671      +/-   ##
===============================================
+ Coverage        59.35%   59.53%   +0.17%     
===============================================
  Files              550      551       +1     
  Lines            21838    21930      +92     
===============================================
+ Hits             12962    13056      +94     
+ Misses            6817     6793      -24     
- Partials          2059     2081      +22

Impacted Files	Coverage Δ
...us/datastructures/utxodiffstore/utxo_diff_store.go	`64.00% <ø> (ø)`
domain/consensus/test_consensus_render_to_dot.go	`0.00% <0.00%> (ø)`
domain/consensus/utils/utxo/immutable_utxo_diff.go	`50.00% <25.00%> (+3.12%)`	⬆️
...sses/consensusstatemanager/add_block_to_virtual.go	`76.74% <50.00%> (+0.55%)`	⬆️
...cesses/consensusstatemanager/reverse_utxo_diffs.go	`65.85% <65.85%> (ø)`
...processes/blockprocessor/validateandinsertblock.go	`70.58% <66.66%> (-0.33%)`	⬇️
...sses/consensusstatemanager/resolve_block_status.go	`72.88% <67.90%> (+0.21%)`	⬆️
...esses/consensusstatemanager/calculate_past_utxo.go	`67.91% <86.66%> (-2.26%)`	⬇️
...sensusstatemanager/test_consensus_state_manager.go	`100.00% <100.00%> (ø)`
domain/consensus/utils/utxo/diff_algebra.go	`100.00% <100.00%> (+13.25%)`	⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28bfc0f...d4e4516. Read the comment docs.

someone235 · 2021-04-13T09:18:55Z

domain/consensus/processes/consensusstatemanager/calculate_past_utxo.go

@@ -66,7 +78,7 @@ func (csm *consensusStateManager) CalculatePastUTXOAndAcceptanceData(stagingArea
 }

 func (csm *consensusStateManager) restorePastUTXO(
-	stagingArea *model.StagingArea, blockHash *externalapi.DomainHash) (externalapi.MutableUTXODiff, error) {
+	stagingArea *model.StagingArea, blockHash *externalapi.DomainHash) (externalapi.UTXODiff, error) {


Why did you convert it to immutable? Doesn't it add extra clone?

Because it is later being passed to a place that expects immutable.
Also, because you shouldn't mutate it outside restorePastUTXO.

ToImmutable doesn't add a clone.

Yeah, but you add an extra clone in applyMergeSetBlocks. I suspect that it was there on purpose because of performance reasons.

domain/consensus/processes/consensusstatemanager/resolve_block_status.go

domain/consensus/utils/utxo/immutable_utxo_diff.go

someone235 · 2021-04-13T13:38:33Z

stability-tests/reorg/reorg.go

@@ -19,6 +19,7 @@ import (
 func testReorg(cfg *configFlags) {
 	params := dagconfig.DevnetParams
 	params.SkipProofOfWork = true
+	params.DisableDifficultyAdjustment = true


Why? If it's a bug please open an issue

SkipProofOfWork without difficulty adjustment creates a ton of blocks with the same timestamp, which kills some assumption in the difficulty adjustment algorithm.
It can't really happen in a real DAG.

svarogg · 2021-04-13T14:43:07Z

domain/consensus/processes/consensusstatemanager/resolve_block_status.go

+		// During resolveSingleBlockStatus, all unverifiedBlocks (excluding the tip) were assigned their selectedParent
+		// as their UTXODiffChild.
+		// Now that the whole chain has been resolved - we can reverse the UTXODiffs, to create shorter UTXODiffChild paths.
+		err = csm.reverseUTXODiffs(stagingArea, unverifiedBlocks, tipUTXOSet, oneBlockBeforeCurrentUTXOSet, useSeparateStagingAreaPerBlock)


If we crash in the middle/after reverseUTXODiffs, but before we committed the main stagingArea, we are going to be left with UTXODiffs to nothing.

svarogg · 2021-04-13T15:02:13Z

stability-tests/reorg/reorg.go

@@ -19,6 +19,7 @@ import (
 func testReorg(cfg *configFlags) {
 	params := dagconfig.DevnetParams
 	params.SkipProofOfWork = true
+	params.DisableDifficultyAdjustment = true


SkipProofOfWork without difficulty adjustment creates a ton of blocks with the same timestamp, which kills some assumption in the difficulty adjustment algorithm.
It can't really happen in a real DAG.

domain/consensus/model/utxo_diff_reversal_data.go

domain/consensus/processes/consensusstatemanager/resolve_block_status.go

someone235 · 2021-04-19T09:18:12Z

domain/consensus/processes/consensusstatemanager/reverse_utxo_diffs.go

+
+	log.Debugf("Reversing utxoDiffs")
+
+	// Set previousUTXODiff and previousBlock to oneBlockBeforeTip before we start touching them,


oneBlockBeforeTip doesn't exist anymore

someone235 · 2021-04-19T09:28:52Z

domain/consensus/processes/consensusstatemanager/reverse_utxo_diffs.go

+	onEnd := logger.LogAndMeasureExecutionTime(log, "reverseUTXODiffs")
+	defer onEnd()
+
+	readStagingArea := model.NewStagingArea()


Maybe rename it to emptyStagingArea?

I prefer "readStagingArea" because it's only used for reads (and the writes happen in a separate staging area).

someone235 · 2021-04-19T15:25:12Z

domain/consensus/processes/consensusstatemanager/reverse_utxo_diffs.go

+		return err
+	}
+
+	// oneBlockBeforeTip is special in the sense that we don't have it's diff available in reverse, however, we


oneBlockBeforeTip doesn't exist anymore

svarogg added 19 commits April 1, 2021 16:19

Use selectedParent instead of selectedTip for non-selectedTip blocks …

f754afd

…in restoreSingleBlockStatus

Cache the selectedParent for re-use in a resolveSingleBlockStatus chain

3a62629

Implement and use reverseUTXOSet

bff3dcd

Reverse blocks in correct order

07723dd

Support resolveBlockStatus without separate stagingAreas for usage of…

4bf1fd5

… testConsensus

Handle the case where the tip of the resolved block is not the next s…

242f39c

…electedTip

Unify isResolveTip

d24916f

Some minor fixes and cleanup

bed7479

Add full finality window re-org test to stability-slow

82ce416

rename: useSeparateStagingAreasPerBlock -> useSeparateStagingAreaPerB…

74d9116

…lock

Better logs in resolveSingleBlockStatus

b7a77d0

A few retouches to reverseUTXODiffs

75fb577

TEMPORARY COMMIT: EXTRAT ALL DIFFFROMS TO SEPARATE METHODS

4fcca1b

TEMPORARY COMMIT: REMOVE DIFFICULTY CHECKS IN DEVNET

c0af9dc

Don't pre-allocate in utxo-algebra, since the numbers are not known a…

7e9614d

…head-of-time

Add some logs to reverseUTXODiffs

500985c

Revert "TEMPORARY COMMIT: REMOVE DIFFICULTY CHECKS IN DEVNET"

04c2080

This reverts commit c0af9dc.

Revert "TEMPORARY COMMIT: EXTRAT ALL DIFFFROMS TO SEPARATE METHODS"

43f964f

This reverts commit 4fcca1b.

Merge remote-tracking branch 'origin/v0.10.0-dev' into utxo-diff-updates

5712338

svarogg linked an issue Apr 11, 2021 that may be closed by this pull request

Big reorgs take a lot of memory #1516

Closed

Merge branch 'v0.10.0-dev' into utxo-diff-updates

4590ac2

someone235 requested changes Apr 13, 2021

View reviewed changes

svarogg added 6 commits April 13, 2021 17:19

Remove redundant paranthesis

88c7752

Revise some logs messages

c247b0a

Rename:oneBlockBeforeCurrentUTXOSet -> lastResolvedBlockUTXOSet

126ae4c

Don't break if the block was resolved as invalid

2807804

rename unverifiedBlocks to recentlyVerifiedBlcks in reverseUTXODiffs

979dc1c

Add errors.New to the panic, for a stack trace

ad5d52c

svarogg commented Apr 13, 2021

View reviewed changes

svarogg added 3 commits April 14, 2021 12:35

Reverse the UTXODiffs after the main block has been commited

3b702df

Use the correct value for previousUTXODiff

aa23e95

Add test for ReverseUTXODiff

199041b

someone235 requested changes Apr 19, 2021

View reviewed changes

svarogg added 3 commits April 19, 2021 16:51

Merge remote-tracking branch 'origin/v0.10.0-dev' into utxo-diff-updates

4d717b3

Fix some names and comments

c1d4acc

Update TestReverseUTXODiffs to use consensus.Config

ca241ce

someone235 requested changes Apr 19, 2021

View reviewed changes

Fix comments mentioning 'oneBlockBeforeTip'

d4e4516

someone235 approved these changes Apr 20, 2021

View reviewed changes

someone235 merged commit dfd8b34 into v0.10.0-dev Apr 20, 2021

someone235 deleted the utxo-diff-updates branch April 20, 2021 07:26

stasatdaglabs mentioned this pull request Apr 20, 2021

Big reorgs take a lot of memory #1516

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new mechanism for updating UTXO Diffs #1671

Implement new mechanism for updating UTXO Diffs #1671

svarogg commented Apr 11, 2021 •

edited

codecov bot commented Apr 11, 2021 •

edited

someone235 Apr 13, 2021

svarogg Apr 13, 2021 •

edited

someone235 Apr 19, 2021

someone235 Apr 13, 2021

svarogg Apr 13, 2021 •

edited

svarogg Apr 13, 2021

svarogg Apr 13, 2021 •

edited

someone235 Apr 19, 2021

someone235 Apr 19, 2021

svarogg Apr 19, 2021

someone235 Apr 19, 2021


		log.Debugf("Reversing utxoDiffs")

		// Set previousUTXODiff and previousBlock to oneBlockBeforeTip before we start touching them,

Implement new mechanism for updating UTXO Diffs #1671

Implement new mechanism for updating UTXO Diffs #1671

Conversation

svarogg commented Apr 11, 2021 • edited

codecov bot commented Apr 11, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

svarogg Apr 13, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svarogg Apr 13, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svarogg Apr 13, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svarogg commented Apr 11, 2021 •

edited

codecov bot commented Apr 11, 2021 •

edited

svarogg Apr 13, 2021 •

edited

svarogg Apr 13, 2021 •

edited

svarogg Apr 13, 2021 •

edited