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

Typed value #244

Merged
Merged

Conversation

uncomputable
Copy link
Collaborator

We need to encode values with padding on the Bit Machine. This encoding requires information about the type of the value. It turns out that universally typing all Simplicity values produces the smallest diff, and it is the safest API. This PR refactors the Value struct to include type information.

I also have ideas for a Word struct that wraps values of the word type. Only these values are safe to use in word jets. I will leave this for a follow-up PR.

This is very old debugging code that shouldn't be there any longer.
This trait lets Boolean iterators collect their bits. Simplicity values
have a compact and a padded bit encoding, so there will be two bit
iterators. BitCollector gives both iterators collection functionality,
without us defining four collection methods for two iterators.

Temporarily add #[allow(dead_code)] so the diff of the coming commits
becomes smaller.
It helps to have dedicated constructors for each word type,
so users of the API don't have to compute 2^n in their head.
Values have two bit encodings:

1) Compact: witness encoding, IMR computation
2) Padded: values on the Bit Machine

The code so far used the compact encoding exclusively, which is
incorrect for sum values on the Bit Machine. As a fix, this commit
introduces the padded encoding. However, a value can only be encoded
with padding if its type is known. To this end, this commit refactors
the Value struct so it knows its Simplicity type.

Adding types to all values instead of "upgrading" a typeless value to a
typed value has several benefits:

1) Bit words, products and options get their type for free when they are
   constructed. The caller doesn't have to supply additional type info.
   This covers almost all values that we are using in the code.
2) Values cannot be decoded without type info. Type checking happens
   implicitly during decoding. The decoded value gets its type for free.
3) Values are used as word constants and as witness data. In both cases,
   we want to check if the supplied value is of the correct type.
   There is (almost?) no use case for untyped values.

The new API exposes two Boolean iterators (iter_compact / iter_padded)
instead of do_each_bit. The iterators are more flexible and can be used
in conjunction with BitCollector.

Most changes in this commit happen inside value.rs. The rest is
renamings of Arc<Value> to Value, etc.
Use from_byte_array when possible. This should be faster than a deep
recursion into ever smaller Value constructors. The code is also more
concise. Update documentation.
@uncomputable
Copy link
Collaborator Author

Rebased on latest master

Copy link
Collaborator

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 5a5a4e2 successfully ran local tests; nice! And does the API changes needed to later replace the Value internal representation with a flat bit array, which should greatly speed things up

@apoelstra apoelstra merged commit 7949187 into BlockstreamResearch:master Aug 29, 2024
16 checks passed
@uncomputable uncomputable deleted the 2024-08-typed-value branch August 30, 2024 11:56
@uncomputable uncomputable mentioned this pull request Sep 11, 2024
apoelstra added a commit that referenced this pull request Sep 11, 2024
69d3e51 fix: Export BitCollector (Christian Lewe)

Pull request description:

  This tiny PR adds an export that I forgot in #244. It turns out that I need this export.

ACKs for top commit:
  apoelstra:
    ACK 69d3e51 successfully ran local tests; SGTM

Tree-SHA512: 9502cbbebe3939a99d7689cf1bba8fe11c1050886d6a4cf5ef61748812be864344dc117592da597157309b0b68918b63d805eda5918cc950dc84cc68f0b389cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants