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

[WIP] Support for variable size MMRs #2690

Closed
wants to merge 1 commit into from

Conversation

antiochp
Copy link
Member

@antiochp antiochp commented Mar 19, 2019

tl;dr Track offset and size per element in the append-only file impl.
Currently track them in memory (and build this when reading the file). But it would be strightforward to persist this size data to disk if necessary (via some kind of size file alongside the data file).

I'd like to keep size data out of the data file itself to keep sync data small (we can rebuild size info on receiving side).

Replaces a big chunk of #1260. The approach in #1260 was to include optional "extra" data in a parallel data file associated with the MMR. This approach here where we track sizes (just in memory currently) is far cleaner and a lot more flexible with not much additional overhead.

If this approach works then it should be relatively straightforward to make fee and lock_height both optional on kernels. We can then explore introducing an optional relative_kernel the same way via a new kernel feature to allow us to support "relative lock heights".

TODO -

  • Get save_prune working - we need to read one file and write to another, taking care to account for the element sizes.
  • Testing, testing, testing.
  • Add optional elmt_size to append-only file to support "fixes size"MMRs
  • Clean up println! logging...
  • Windows testing (file release etc.)

@antiochp
Copy link
Member Author

Ok, trying to leverage this to allow variable size entries in the header MMR was a dumb idea.
HeaderEntry is currently a fixed 53 bytes (32 byte hash + difficulty and timestamp data).
If we swap these out for full headers the size of the header MMR increases significantly (due to the variable size PoW solution nonces).

Fixed size header entries: 5MB (header MMR data file)
Full variable size headers: 36MB (header MMR data file)

This does not mean the variable size support is a bad idea. Just that we don't necessarily want to do this for the header MMR.

We do still want to support variable entries in the kernel MMR.

@DavidBurkett
Copy link
Contributor

Aside from header sync, which can use the DB, I can't fathom a reason we would ever need the PoW after it's been verified. I see no reason to support full HEADERS w/pow in the MMRs.

@antiochp
Copy link
Member Author

Aside from header sync, which can use the DB, I can't fathom a reason we would ever need the PoW after it's been verified. I see no reason to support full HEADERS w/pow in the MMRs.

The main reason would be simplicity -

  • We header stored in the MMR would be the same as the header stored in the db.
  • We would not necessarily need to store them in the db if we could simply store full headers in the MMR itself.

It would just be one less serialization format to need to be concerned with.

@antiochp
Copy link
Member Author

antiochp commented Mar 25, 2019

We need to store the header hash in the header MMR to allow the header MMR to be used as our "header by height" index. We can efficiently get the hash of the header at height 1,000,000 by simply reading the data in the header MMR at pos 1,000,000.

We extended this to include the difficulty and timestamp info because we wanted to be able to leverage the header MMR when validating the difficulty (we need to read the previous 60 headers to do so).
Vie the db this involves 60 reads to pull the headers.
Via the header MMR it is (in theory) a read of a single contiguous chunk of bytes from the underlying mmap.

It turns out the 60 reads from LMDB are still faster given the internal impl of LMDB (and its own underlying mmap).

(Just some context around why we store anything in the header MMR).

@antiochp antiochp marked this pull request as ready for review April 5, 2019 12:26
Support fixed size data file via an optional elmt_size.
Support variable size data file via optional size_file.
@antiochp antiochp force-pushed the variable_size_mmr branch from c49ec7e to 67cf3b6 Compare April 5, 2019 12:33
@antiochp antiochp closed this Apr 5, 2019
@antiochp
Copy link
Member Author

antiochp commented Apr 5, 2019

Closing - replaced with #2733 which is rebased cleanly on master.

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

Successfully merging this pull request may close these issues.

2 participants