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

3. refactor(db): add disk serialization types for transactions #3741

Merged
merged 7 commits into from
Mar 9, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Mar 4, 2022

Motivation

We want to:

To do that, we need to have serialization-specific types for transactions and transparent outputs. And we want to remove duplicate code for height, transaction, and transparent serialization.

Designs

RocksDB section of the state RFC:
https://github.com/ZcashFoundation/zebra/blob/lightwalletd-state-rfc-fix/book/src/dev/rfcs/0005-state-updates.md#rocksdb-data-structures

Some of these types are renamed in this PR, for consistency.

Solution

Code changes:

  • simplify block height serialization
  • create a TransactionIndex type, and use it in TransactionLocation
  • create transparent OutputIndex and OutputLocation types

The disk serialization format is the same, but some of the type names have changed. (These names don't get serialized.)

RFC updates:

  • make transparent database type names consistent
  • fix a bug in the Utxo.is_coinbase derivation

Review

This PR is based on #3717, GitHub and Mergify should automatically handle the rebase and merge.

Reviewer Checklist

  • Code implements design
  • Existing tests pass
  • Snapshot updates don't change data

Follow Up Work

@teor2345 teor2345 added A-docs Area: Documentation A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium ⚡ A-state Area: State / database changes labels Mar 4, 2022
@teor2345 teor2345 requested a review from a team as a code owner March 4, 2022 08:51
@teor2345 teor2345 self-assigned this Mar 4, 2022
@teor2345 teor2345 requested review from upbqdn and removed request for a team March 4, 2022 08:51
@teor2345 teor2345 changed the title refactor(db): add disk serialization types for transactions 3. refactor(db): add disk serialization types for transactions Mar 4, 2022
@teor2345 teor2345 requested review from jvff and removed request for upbqdn March 4, 2022 09:11
@codecov
Copy link

codecov bot commented Mar 4, 2022

Codecov Report

Merging #3741 (b8e96c0) into main (44c7b5d) will decrease coverage by 0.25%.
The diff coverage is 66.39%.

@@            Coverage Diff             @@
##             main    #3741      +/-   ##
==========================================
- Coverage   79.00%   78.74%   -0.26%     
==========================================
  Files         292      292              
  Lines       33311    33379      +68     
==========================================
- Hits        26317    26285      -32     
- Misses       6994     7094     +100     

dconnolly
dconnolly previously approved these changes Mar 4, 2022
Copy link
Contributor

@dconnolly dconnolly left a comment

Choose a reason for hiding this comment

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

lgtm

@teor2345 teor2345 force-pushed the disk-format-split-modules branch from f706967 to 58628f5 Compare March 4, 2022 22:43
@teor2345 teor2345 requested a review from a team as a code owner March 4, 2022 22:43
@teor2345 teor2345 requested review from conradoplg and removed request for a team March 4, 2022 22:43
@teor2345 teor2345 force-pushed the db-transaction-types branch from c2d735d to 79feace Compare March 4, 2022 22:43
@teor2345 teor2345 removed request for jvff and conradoplg March 6, 2022 21:16
@teor2345 teor2345 force-pushed the disk-format-split-modules branch from 58628f5 to bf9de1e Compare March 6, 2022 21:20
@teor2345 teor2345 force-pushed the db-transaction-types branch from 79feace to 362879e Compare March 6, 2022 21:20
@teor2345 teor2345 force-pushed the disk-format-split-modules branch from bf9de1e to 0c84f37 Compare March 7, 2022 03:26
@teor2345 teor2345 force-pushed the db-transaction-types branch from 362879e to d989daa Compare March 7, 2022 03:26
@conradoplg conradoplg force-pushed the disk-format-split-modules branch from 0c84f37 to 0698e4b Compare March 7, 2022 14:53
@teor2345 teor2345 force-pushed the disk-format-split-modules branch from 0698e4b to 0c5d549 Compare March 8, 2022 02:56
@teor2345 teor2345 force-pushed the db-transaction-types branch from d989daa to c4dc5cc Compare March 8, 2022 02:56
Base automatically changed from disk-format-split-modules to main March 8, 2022 07:59
mergify bot added a commit that referenced this pull request Mar 8, 2022
@mergify mergify bot merged commit 081cda7 into main Mar 9, 2022
@mergify mergify bot deleted the db-transaction-types branch March 9, 2022 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation A-rust Area: Updates to Rust code A-state Area: State / database changes C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants