More error correction preparation (upgrades to Field, Polynomial, InvalidResidueError and others)
#202
+1,060
−329
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is purely a code refactor to get rid of a `use` statement (which will trigger an "unused code" warning if the macro is ever called somewhere where the Field trait is already in scope). As a side effect it reduces the size of the macro by roughly 75% in terms of line count.
Several methods (and we are going to add more) don't really belong in a general-purpose field trait, which really is just "a type that can be multiplied and added and has some associated constants". This isn't a general-purpose math library, but fields are a pretty common thing for people to want. And it's pretty easy for us to expose this trait in a nice way, so we might as well do it.
This will make it easier for PrintImpl to output error correction parameters. Since this is an implementation detail of the library, stick it into the Bech32Field trait rather than the Field one.
In the next commits we'll introduce a generic no-alloc container called FieldVec whose implementation will be much nicer if we have a Default bound on all of our field elements. This way we can implement the "FieldVec", which despite the name really has nothing to do with fields, purely in terms of core traits, rather than confusing readers of the code by having Field bounds everywhere without ever doing algebra. Since fields all have a ZERO member, which is a sensible output for Default, this bound is reasonable to require.
This adds some new methods to Field, which have default impls and are therefore non-breaking. (And they are general-purpose "field" things.)
This will allow `Polynomial` to work without an allocator (at least for "small" checksums, including every checksum supported by this library). Here a "small" checksum is one of length at most 6 (which covers bech32 and bech32m). codex32 (13 characters) and "long codex32" (15 characters) will not work with no-alloc. I would kinda like to fix this but it results in large types, especially a large InvalidResidueError type, so probably it will need to be feature-gated or something. For now we just punt on it.
We can now use `PrintImpl` even without an allocator. This is of limited use of course, but it paves the ground for use to do error correction without an allocator, which is interesting.
We are going to want to extend the ChecksumError::InvalidResidue error variant to allow error correction, and in order to do so, we need to know the actual residue computed from a string, not just that the residue failed to match the target. Uses the `Polynomial` type; see the previous commits for some caveats about this type when alloc is disabled. (Prior to this PR, you just couldn't use Polynomial at all without alloc.) As an initial application of the new error, avoid re-validating a bech32 address to handle the distinction between bech32m and bech32. Instead, if bech32m validation fails, check if the "bad" residue actually matches the bech32 target. If so, accept. On my system this reduces the `bech32_parse_address` benchmark from 610ns to 455ns, more than a 33% speedup.
apoelstra
force-pushed
the
2024-09--fieldvec
branch
from
d61cb0a
to
09bcf08
Compare
|
CI failed because I had a doccomment referring to a field of But now this PR is way too big :). Gonna close it and split it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has a fairly large diff but the commits should be self-contained and much of the diff consists of comments. In particular this introduces a new type
FieldVecto backPolynomial, which strives to provide a sensible API even in a no-alloc case:FieldVectype panics for large checksums, but since it is only (planned to be) used in error correction, it should be easy to detect this up front and provide a sensible error to the user.The threshold for "large" is currently set at "bigger than bech32", but it is easy to change this and even easy to add a feature-gate.
I believe this completes all the changes to the traits needed for error correction, and the next PR can just implement correction.