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

Fix to also remove VoterParams and LastProofHash in PruneStates #502

Merged
merged 3 commits into from
Nov 24, 2022

Conversation

Mdaiki0730
Copy link
Member

@Mdaiki0730 Mdaiki0730 commented Nov 15, 2022

Description

Function LoadVoters(), if valInfo is pruned at the input height, the function will return directly without querying voteParams and proofHash at the end of this function, which makes the permanent persistence of voteParams and proofHash useless.

I would prefer to prune the aforementioned VoterParams and LastProofHash in the function PruneStates()

PR Check List

  • If you intend to keep these as history, we won't need this implementation.

@Mdaiki0730 Mdaiki0730 added the C: enhancement Classification: New feature or its request, or improvement in maintainability of code label Nov 15, 2022
@Mdaiki0730 Mdaiki0730 self-assigned this Nov 15, 2022
@codecov
Copy link

codecov bot commented Nov 15, 2022

Codecov Report

Merging #502 (46de8e5) into main (e6f57a1) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #502      +/-   ##
==========================================
+ Coverage   65.39%   65.48%   +0.08%     
==========================================
  Files         278      278              
  Lines       37995    38003       +8     
==========================================
+ Hits        24846    24885      +39     
+ Misses      11328    11309      -19     
+ Partials     1821     1809      -12     
Impacted Files Coverage Δ
state/store.go 53.45% <100.00%> (+2.07%) ⬆️
crypto/sr25519/pubkey.go 35.89% <0.00%> (-7.70%) ⬇️
mempool/reactor.go 78.57% <0.00%> (-1.10%) ⬇️
p2p/switch.go 65.08% <0.00%> (-0.83%) ⬇️
p2p/conn/connection.go 79.80% <0.00%> (-0.20%) ⬇️
consensus/state.go 73.67% <0.00%> (+0.40%) ⬆️
consensus/reactor.go 75.37% <0.00%> (+1.14%) ⬆️
blockchain/v0/pool.go 79.58% <0.00%> (+1.55%) ⬆️
privval/signer_listener_endpoint.go 91.26% <0.00%> (+2.38%) ⬆️
... and 2 more

@Mdaiki0730 Mdaiki0730 force-pushed the main-fix-prunestates branch 2 times, most recently from 65c4a43 to fd4a2df Compare November 22, 2022 06:27
@Mdaiki0730 Mdaiki0730 marked this pull request as ready for review November 24, 2022 02:32
@tnasu tnasu added C: bug Classification: Something isn't working and removed C: enhancement Classification: New feature or its request, or improvement in maintainability of code labels Nov 24, 2022
@Mdaiki0730 Mdaiki0730 merged commit cccc128 into Finschia:main Nov 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: bug Classification: Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants