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

[Core] optimize the resync process #3540

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Jim8y
Copy link
Contributor

@Jim8y Jim8y commented Oct 18, 2024

Description

Previously we have two sync mode, 1. resync with all witness verified, its slow; 2. resync without witness being verified, fast but sort of unsecure. This pr tries to optimize the resync process to achieve the speed of unverify while remaining it as verifiable.

Fixes # (issue)

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Test Configuration:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 18, 2024

@roman-khimov @shargon this pr is for the resync optimization, it still verify blocks, but can achieve the speed of unverify.

Consensus change is not frequent, here is the full list of consensus history:

0 NVg7LjGcUSrgxgjX3zEgqaksfMaiS8Z6e1
171486 NSaJ4BHAcTjosi2HvWeDwmGdC5SFDhFEi6
171507 NVg7LjGcUSrgxgjX3zEgqaksfMaiS8Z6e1
171801 NYkw1YaCPHB4BoTFDdLYMXXG84P6xKd8cz
173817 Ngu1EFHZ8dkDYrW7ZzYkt5cNpD8Dj1gANN
173922 NYkw1YaCPHB4BoTFDdLYMXXG84P6xKd8cz
200655 NSUbQSuHFV7tKv1qYnE2YroVWgFJN2GXiv
200676 NYkw1YaCPHB4BoTFDdLYMXXG84P6xKd8cz
201075 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
240324 NVJVJuxyNjpFFfQ1FBbjn9jishs5WM4CKA
243831 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
255717 NYkw1YaCPHB4BoTFDdLYMXXG84P6xKd8cz
255738 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
595980 NY1Rp2M7CBipYezLzGgf9yc2JVLtc181H5
596001 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
1350216 NSEE58vrYk1851vx4qastbhApmZSyeEowG
1368906 Nid1QZYU4AZQhgT4PYjx9mPY6Eis3YMomB
1370901 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
1385034 NNVpmAqvEHSKKXQq9VXXB1F7dEfJ1EvZh9
1440957 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
2140131 Nid1QZYU4AZQhgT4PYjx9mPY6Eis3YMomB
2580228 NcXMxBSmRJUqgPgKbntZtxaFAZvzk2b485
2655156 Nid1QZYU4AZQhgT4PYjx9mPY6Eis3YMomB
2939202 NSiVJYZej4XsxG5CUpdwn7VRQk8iiiDMPM
4589634 NhhCW7yCTH8jHdabDGkRXr8a3nkpwj5FBY
4939662 NPyVyp2QdRdqWAvkjipVjiy98ZNQJ4LmaU

@Jim8y Jim8y marked this pull request as draft October 18, 2024 03:38
@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 18, 2024

Only the block whose Nextconsensu is changed will be verified by witness, rest of them will be verified with prehash.

According to the chain of hash, only if someone can break the Sha256, can it create a valid blockhash that can pass tthis verification.

@Hecate2
Copy link
Contributor

Hecate2 commented Oct 18, 2024

Should still execute block.Verify for the genesis block. Not executed in current code.

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 18, 2024

.

This is a demo, will also add the consensus history as checkpoint.

@dusmart
Copy link

dusmart commented Oct 18, 2024

This is a demo, will also add the consensus history as checkpoint.

Yeah. The checkpoint will eliminate this attack method: no consensus change at all, with a totally faked chain.

@roman-khimov
Copy link
Contributor

I still don't understand how checking signatures in one place and not checking in another improves anything. Exactly because of

this attack method: no consensus change at all, with a totally faked chain

Checkpoints are OK, but as an index->hash map, and you don't need many of them I think (mostly it'd be a single one). The main value of checkpoints to me is that they can be used to avoid messing with old blocks completely, just fetch state/blocks from this known-good block (#3463).

@Jim8y
Copy link
Contributor Author

Jim8y commented Oct 18, 2024

I still don't understand how checking signatures in one place and not checking in another improves anything. Exactly because of

this attack method: no consensus change at all, with a totally faked chain

Checkpoints are OK, but as an index->hash map, and you don't need many of them I think (mostly it'd be a single one). The main value of checkpoints to me is that they can be used to avoid messing with old blocks completely, just fetch state/blocks from this known-good block (#3463).

I am also thinking about what is the best way of doing this. Could be index-> hash map, chould be nextconsensus update history, looks all work for me. As is reminded by @dusmart, hardcode the checkpoint to the core can actually defend the Long Range Attack.

@@ -163,7 +166,8 @@ public static ProtocolSettings Load(IConfigurationSection section)
InitialGasDistribution = section.GetValue("InitialGasDistribution", Default.InitialGasDistribution),
Hardforks = section.GetSection("Hardforks").Exists()
? EnsureOmmitedHardforks(section.GetSection("Hardforks").GetChildren().ToDictionary(p => Enum.Parse<Hardfork>(p.Key, true), p => uint.Parse(p.Value))).ToImmutableDictionary()
: Default.Hardforks
: Default.Hardforks,
CheckPoint = section.GetSection("CheckPoint").GetChildren().Select(p => p.Value).ToList()
Copy link
Member

Choose a reason for hiding this comment

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

You could make a class for this json to be bind on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants