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

Implement pruning point proof #1832

Merged
merged 41 commits into from
Oct 26, 2021

Conversation

someone235
Copy link
Collaborator

@someone235 someone235 commented Oct 17, 2021

Implements kaspanet/research#3

…number of connections in the integration test to 32
@codecov
Copy link

codecov bot commented Oct 17, 2021

Codecov Report

Merging #1832 (9ba63c2) into v0.11.0-dev (afaac28) will decrease coverage by 0.05%.
The diff coverage is 68.52%.

Impacted file tree graph

@@               Coverage Diff               @@
##           v0.11.0-dev    #1832      +/-   ##
===============================================
- Coverage        62.17%   62.11%   -0.06%     
===============================================
  Files              621      619       -2     
  Lines            28477    28748     +271     
===============================================
+ Hits             17705    17857     +152     
- Misses            8307     8370      +63     
- Partials          2465     2521      +56     
Impacted Files Coverage Δ
...ocesses/dagtraversalmanager/dagtraversalmanager.go 77.58% <ø> (+2.58%) ⬆️
domain/consensus/ruleerrors/rule_error.go 87.87% <ø> (ø)
domain/consensus/test_consensus_render_to_dot.go 0.00% <0.00%> (ø)
.../network/netadapter/server/grpcserver/p2pserver.go 69.69% <ø> (ø)
...rotocol/flows/blockrelay/ibd_with_headers_proof.go 42.36% <37.50%> (-2.89%) ⬇️
.../consensus/processes/dagtraversalmanager/window.go 69.23% <60.00%> (-1.27%) ⬇️
...ocesses/pruningproofmanager/pruningproofmanager.go 62.02% <62.02%> (ø)
.../pruning_violation_proof_of_work_and_difficulty.go 68.25% <63.63%> (-5.66%) ⬇️
domain/consensus/test_consensus_getters.go 72.22% <71.42%> (-2.07%) ⬇️
domain/consensus/consensus.go 68.91% <74.35%> (-0.49%) ⬇️
... and 28 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 afaac28...9ba63c2. Read the comment docs.

@@ -10,8 +10,8 @@ import (
)

func Test64IncomingConnections(t *testing.T) {
// Much more than 64 hosts creates a risk of running out of available file descriptors for leveldb
const numBullies = 64
// Much more than 32 hosts creates a risk of running out of available file descriptors for leveldb
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the number in this comment.

@@ -174,6 +174,8 @@ type Params struct {

// BaseSubsidy is the starting subsidy amount for mined blocks.
BaseSubsidy uint64

PruningProofM uint64
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment explaining what this is.

@@ -91,6 +91,7 @@ func (c *ConnectionManager) Stop() {
}

c.loopTicker.Stop()
c.run()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This forces the next iteration in the connection loop, so you won't have to wait 30 seconds before the connection loop stops.

ErrPruningProofSelectedTipIsNotThePruningPoint = newRuleError("ErrPruningProofSelectedTipIsNotThePruningPoint")
ErrPruningProofInsufficientBlueWork = newRuleError("ErrPruningProofInsufficientBlueWork")
ErrPruningProofMissingBlockAtDepthMFromNextLevel = newRuleError("ErrPruningProofMissingBlockAtDepthMFromNextLevel")
ErrPruningProofMissesBlocksBelowPruningPoint = newRuleError("ErrPruningProofMissesBlocksBelowPruningPoint")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weren't these elsewhere before? Why did you copy them here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? I didn't copy them

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, brain fart. Nevermind.

return nil, err
}

if !isNotFoundError {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any other time except for the virtual genesis when the current block would not be found?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is that we always prefer to use the data from daaWindowStore if available, because when we import blocks with trusted data we change their ghostdata data and drop from the merge set every pruned header. So if we rely on the ghostdag data of blocks with trusted data when calculating difficulty we'll get the wrong difficulty.

}

visited.Add(current)
isAncestorOfPruningPoint, err := ppm.dagTopologyManagers[blockLevel].IsAncestorOf(stagingArea, current, selectedTip)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selected tip is not necessarily the pruning point. Please rename this variable.

@@ -91,7 +91,7 @@ func (c *ConnectionManager) Stop() {
}

c.loopTicker.Stop()
// Force the next iteration so the connection loop will stop immediately and not after `connectionsLoopInterval`.
// Force the next iteration so the connection loop will stop immediately and not after `connectionsLoopInterval`ErrPruningProofMissesBlocksBelowPruningPoint.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy+paste error.

stasatdaglabs
stasatdaglabs previously approved these changes Oct 26, 2021
@someone235 someone235 merged commit 5dbb1da into kaspanet:v0.11.0-dev Oct 26, 2021
@someone235 someone235 deleted the build-dag-per-level branch October 26, 2021 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants