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

feat: check block filter hashes #132

Merged
merged 6 commits into from
Mar 24, 2023

Conversation

yangby-cryptape
Copy link
Collaborator

@yangby-cryptape yangby-cryptape commented Mar 15, 2023

Commits

  • refactor: move some data from peer state to peer itself

    • Why? Some data are not belongs to the peer state, when state was changed, those data should keep as before.

    • N.B. Rename many peer to peer_index since peer and peer_index are different things.

  • refactor: refactor peer state to be a state machine

    • Why? It's hard to use a lot of .some() to distinguish the peer state.

      The complexity of that struct grows exponentially.

  • refactor: better judgment for timeout

    Remove several TODO.

  • chore(deps): bump ckb from 0.106.0 to 0.108.0

  • feat: check block filter hashes

    • First, the light client will try to connect more than max_outband_peers peers.

    • If a check point was confirmed by more than max_outband_peers / 2 peers and there are more than check_point_interval blocks between it and the last proved number, we finalized it and store it in to disk.

      N.B. A finalized check point could not roll back! Since it's a long fork.

    • After a check point was finalized, for each peer, light client will maintain a list of block filters between that check point and the last proved number.

      Denote those lists as [L] (called as "latest block filter hashes" in the code).

    • If light client want to get block filters for blocks which start number is N:

      • If N is less than the finalized check point number, and it is between the check point M and check point M+1, light client will request the hashes between check point M and check point M+1 (called as "cached block filter hashes" in the code).
        After all hashes were downloaded, then light client will request the block filters and checks them with those hashes.

      • If N is greater than the finalized check point number, then light client will only request the block filters which were confirmed by more than max_outband_peers / 2 peers in [L], and check those block filters with hashes in [L].

TODO

  • Unit tests are not enough.

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

Patch coverage: 45.73% and project coverage change: -9.64 ⚠️

Comparison is base (98dfe2a) 79.35% compared to head (e51a708) 69.71%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #132      +/-   ##
===========================================
- Coverage    79.35%   69.71%   -9.64%     
===========================================
  Files           23       26       +3     
  Lines         4950     6373    +1423     
===========================================
+ Hits          3928     4443     +515     
- Misses        1022     1930     +908     
Flag Coverage Δ
unittests 69.71% <45.73%> (-9.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/main.rs 14.28% <ø> (ø)
...er/components/block_filter_check_points_process.rs 0.00% <0.00%> (ø)
...s/filter/components/block_filter_hashes_process.rs 0.00% <0.00%> (ø)
src/service.rs 64.73% <ø> (ø)
src/subcmds.rs 0.00% <0.00%> (ø)
src/protocols/light_client/mod.rs 69.12% <33.69%> (-12.75%) ⬇️
src/protocols/filter/block_filter.rs 59.28% <42.56%> (-27.15%) ⬇️
...light_client/components/send_transactions_proof.rs 88.23% <45.45%> (-3.07%) ⬇️
src/protocols/light_client/peers.rs 60.95% <46.54%> (-24.55%) ⬇️
...otocols/filter/components/block_filters_process.rs 74.87% <68.27%> (-20.74%) ⬇️
... and 7 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yangby-cryptape yangby-cryptape marked this pull request as ready for review March 16, 2023 05:38
@yangby-cryptape yangby-cryptape force-pushed the pr/fix-tx-hidden-issue branch from 5ec5fd4 to c2a005f Compare March 16, 2023 09:09
@quake quake self-requested a review March 17, 2023 00:01
@zhangsoledad zhangsoledad mentioned this pull request Mar 22, 2023
1 task
Copy link
Member

@quake quake left a comment

Choose a reason for hiding this comment

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

this PR's code logic looks good to me, leave only some code style comments.

src/protocols/light_client/peers.rs Outdated Show resolved Hide resolved
src/protocols/light_client/peers.rs Show resolved Hide resolved
src/protocols/light_client/peers.rs Outdated Show resolved Hide resolved
src/protocols/light_client/peers.rs Outdated Show resolved Hide resolved
src/protocols/light_client/peers.rs Outdated Show resolved Hide resolved
src/protocols/light_client/mod.rs Outdated Show resolved Hide resolved
src/protocols/light_client/mod.rs Outdated Show resolved Hide resolved
src/protocols/light_client/mod.rs Outdated Show resolved Hide resolved
src/protocols/light_client/mod.rs Outdated Show resolved Hide resolved
src/protocols/light_client/mod.rs Outdated Show resolved Hide resolved
@yangby-cryptape yangby-cryptape added the hold Put this pull request on hold. label Mar 23, 2023
@yangby-cryptape yangby-cryptape removed the hold Put this pull request on hold. label Mar 23, 2023
@quake quake merged commit 542cbc6 into nervosnetwork:develop Mar 24, 2023
@yangby-cryptape yangby-cryptape deleted the pr/fix-tx-hidden-issue branch June 30, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants