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

Load sapling chain value into memory #3684

Merged
merged 3 commits into from Nov 17, 2018

Conversation

5 participants
@str4d
Copy link
Contributor

str4d commented Nov 16, 2018

CBlockIndex::nSaplingValue has been correctly set and written to disk since before Sapling activated, meaning that all nodes now are correctly tracking the Sapling shielded pool value on-disk. However, on restart the per-block values are not being read into memory, and so the in-memory pool value appears to be zero on every restart. Setting nSaplingValue in-memory during block index loading fixes the problem.

str4d added some commits Nov 16, 2018

@str4d str4d added bug Sapling labels Nov 16, 2018

@str4d str4d added this to the v2.0.2 milestone Nov 16, 2018

@str4d str4d requested review from bitcartel and LarryRuane Nov 16, 2018

@str4d str4d added this to In Review in Zcashd Team Nov 16, 2018

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Nov 16, 2018

@str4d Can we add a quick test for non-zero value? I think in wallet_persistence.py after # Restart the nodes could work.

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Nov 16, 2018

@bitcartel I pushed an update to the RPC test you suggested. I confirmed that the test now fails without the fix in this PR.

@bitcartel
Copy link
Contributor

bitcartel left a comment

utACK

@Eirik0

Eirik0 approved these changes Nov 17, 2018

Copy link
Contributor

Eirik0 left a comment

utACK

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Nov 17, 2018

@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Nov 17, 2018

📌 Commit 4e11757 has been approved by bitcartel

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Nov 17, 2018

⌛️ Testing commit 4e11757 with merge a47e1fd...

zkbot added a commit that referenced this pull request Nov 17, 2018

Auto merge of #3684 - str4d:load-sapling-chain-value, r=bitcartel
Load sapling chain value into memory

`CBlockIndex::nSaplingValue` has been correctly set and written to disk since before Sapling activated, meaning that all nodes now are correctly tracking the Sapling shielded pool value on-disk. However, on restart the per-block values are not being read into memory, and so the in-memory pool value appears to be zero on every restart. Setting `nSaplingValue` in-memory during block index loading fixes the problem.
@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Nov 17, 2018

☀️ Test successful - pr-merge
Approved by: bitcartel
Pushing a47e1fd to master...

@zkbot zkbot merged commit 4e11757 into zcash:master Nov 17, 2018

1 check passed

homu Test successful
Details

Zcashd Team automation moved this from In Review to Released (Merged in Master) Nov 17, 2018

@daira

This comment has been minimized.

Copy link
Contributor

daira commented Nov 19, 2018

Post-hoc ut(ACK+cov).

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