change(rpc): Update getaddressbalance RPC to return received field
#9295
Conversation
arya2
added
S-blocked
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🔴 ⛓️ Depends-On RequirementsThis rule is failing.Requirement based on the presence of
|
…uct, `PruneTrees`, and moves the logic for tree deduplication to the trait impl
…des to use new trait
- Avoids duplicate validation of format upgrades at startup when db is already upgraded, - Minor refactors - Doc fixes and cleanups
arya2
force-pushed
the
db-format-upgrade-trait
branch
from
55b3602 to
deff106
Compare
- Adds received balances to non-finalized state, and - Applies the partial chain received balance change to the finalized received balance when reading address balances.
| // Return early if there are no blocks in the state and the database is empty. | ||
| // Note: The caller should (and as of this writing, does) return early if the database is empty anyway. | ||
| if initial_tip_height.is_min() { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
This condition is checking if the state contains only the genesis block, not if it's entirely empty.
| // Return early if there are no blocks in the state and the database is empty. | |
| // Note: The caller should (and as of this writing, does) return early if the database is empty anyway. | |
| if initial_tip_height.is_min() { | |
| return Ok(()); | |
| } |
| // Read the address balance from disk. | ||
| let mut balance = db | ||
| .address_balance_location(address) | ||
| .expect("should have address balances in finalized state"); | ||
|
|
||
| // Update the address balance with the tallied received balance. | ||
| *balance.received_mut() = received; |
There was a problem hiding this comment.
I'm not fully convinced of the correctness of the flow here.
What if:
- An AddressBalanceReceived (ABR) is read, and its
receivedis updated in memory - A block is comitted, and another field of the same ABR is updated (e.g.
balance) - The in-memory ABR is written to the DB, overwriting the updated balance
- Below, it is noticed that a block is committed, and the loop continues, but it won't restore the correct balance AFAICT?
I feel like we can't avoid reading and writing to the same field unless it's inside the same database commit.
There was a problem hiding this comment.
This explains the values in the failed test logs, I'm thinking I'll add a mechanism for pausing block commits before writing to disk from an in-place upgrade.
Alternatives:
- add a mechanism for blocking the state service initialization at startup on the completion of certain format upgrades
- remove the in-place upgrade and make it a major format upgrade
There was a problem hiding this comment.
Yes, it seems a blocking format upgrade might be useful here, though a bit annoying for operators.
Another alternative would be to use a new column, but that would add redundancy.
Another option: https://github.com/facebook/rocksdb/wiki/Merge-Operator ?
arya2
force-pushed
the
db-format-upgrade-trait
branch
from
deff106 to
a9a17e2
Compare
Motivation
Updates the
getaddressbalanceRPC to accept either a single address string or a list of them to match zcashd, and adds thereceivedfield on the method's response.This PR is still a draft as it will need an acceptance test.
Depends-On: #9263
Will close #8452.
Solution
receivedfield, and adds an in-place format upgrade,AddressStringsas aVecwith one item, andreceivedfield to thegetaddressbalanceRPC output.Tests
This PR still needs an acceptance test for the db format upgrade.
The changes to the RPC should be covered by existing snapshot tests.
PR Checklist
C-exclude-from-changeloglabel.