Conversation
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (4)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. Walkthrough
ChangesField plaintext support
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Sender
participant EcdhMask
participant Ecdh
participant Recipient
Sender->>EcdhMask: encryptField(recipientPk, plaintext, ephemeralScalar, domain)
EcdhMask->>Ecdh: derive shared point
EcdhMask->>EcdhMask: derive fieldKdf mask
EcdhMask-->>Sender: Ciphertext
Recipient->>EcdhMask: decryptField(ciphertext, recipientScalar, domain)
EcdhMask->>Ecdh: recover shared point
EcdhMask->>EcdhMask: subtract fieldKdf mask
EcdhMask-->>Recipient: Field plaintext
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The field encryption APIs preserve the existing bounded path and include focused round-trip and domain-separation coverage. No merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
A rabbit hops through fields of light Comment |
0xisk
force-pushed
the
feat/ecdh-mask-field-kdf
branch
from
September 17, 2026 06:49
c7fcd25 to
e7732f0
Compare
0xisk
added this pull request to stack #914
September 17, 2026 06:50
`kdf` derives into `[0, 2^248)`, which hides a `Uint<128>` with about 2^-120 slack but leaves the high bits of a full-width `Field` in the clear. Widening `kdf` was not an option: its output is a published ciphertext component, so `fieldKdf` is a second derivation instead. It hashes two halves under distinct half indices and combines them as `k1 + k2 * 2^248 mod p`, landing within 2^-240 of uniform, and its three-element preimage separates it from `kdf` under the same `(sShared, domain)`. `encryptField` / `decryptField` reuse the existing `Ciphertext` struct and reach the weak-input guards through `crypto/Ecdh` exactly as `encrypt` does. Refs: #735
* Width: each half stays inside the 248-bit `degradeToTransient` range, the halves are independent hash queries, and the combined mask leaves that range for most shared points. `encryptField` of a fixed plaintext spreads across the whole field, which is the property `kdf` cannot give. * Round-trip at the field boundaries (zero, 2^248, 2^253, p-1) and as a property over arbitrary keys, ephemerals and plaintexts. * Multi-field: two fields under one key agreement and one tag each; the same tag twice publishes their difference. * `kdf` gains a regression pinning its output below 2^248, so a later widening cannot silently change `encrypt`. Refs: #735
0xisk
force-pushed
the
feat/ecdh-mask-field-kdf
branch
from
September 17, 2026 06:59
e7732f0 to
2c06e4e
Compare
8 tasks
Group the byte and field conversions under the field they belong to, with names from the literature in place of the standard library's degradeToTransient and upgradeFromTransient, which hide a truncation. * truncatedLEOS2IP and truncatedI2LEOSP wrap the two standard library conversions and document that both drop everything past 248 bits. * fromUniformBytes is the field half of RFC 9380 hash_to_field: LEOS2IP_512(tv) mod q, the same reduction as Fq::from_uniform_bytes in Midnight's curves library. * MockFq keeps every circuit impure so the artifact ships with ZKIR and proving keys. Refs: #735
Give hashing to the field its own module, shaped after RFC 9380 hash_to_field, so the construction can be reviewed against a reference step by step. * hash/Sha256 exports digest and hashToField. hashToField follows the RFC's steps and names; its expander is a counter expander in place of expand_message_xmd, and the deviations are listed on the circuit. * hash/IHasher holds the two signatures as a contract type, so a second hash module can be swapped in by changing an import. * MockSha256 implements the interface, so a drifting signature fails compilation. Refs: #735
EcdhMask no longer calls persistentHash or degradeToTransient directly. kdf and pointDigest go through Sha256.digest and Fq.truncatedLEOS2IP, and their outputs are unchanged. fieldKdf now derives through Sha256.hashToField, which reduces all 64 bytes of the two blocks where the inline version reduced 62. Its output changes, the uniformity bound tightens from 2^-241 to 2^-257, and the three Field pad circuits grow by 580 rows at the same k. The fieldKdf tests still pin the old 31-byte halves through MockEcdhMask.fieldKdfHalves and are expected to fail until they are updated. Refs: #735
0xisk
marked this pull request as draft
September 17, 2026 14:27
Mocks sit beside the module they expose, as the Fq mock already does under curves/bls12_381/test. Only the two import paths change.
fromUniformBytes and leos2ipModQ both evaluate a two-digit radix representation in the field. fromRadixDigits now holds that one line with its HAC reference, so the mod q is visible in one place instead of implied by two identical comments. No behaviour change; rows unchanged. Docs: verb-first titles with the formula, backend notes on the truncated pair, every notation term linked to its source, sage checks on the three literals, and @Constraints re-measured through MockFq. The previous values did not come from the mock.
Each circuit names its hash_to_field step and links the FIPS 180-4 and RFC 9380 sections it follows. Titles are verb-first with the formula.
Expected values are inlined per test, computed with an independent Python reference (hashlib for SHA-256, py_ecc for the field), plus formula checks done in TypeScript so no vector file is needed.
kdf pads stay below 2^248, so a ciphertext of a 248-bit or Field value never wraps and its top bits are readable. The spec measures it: a threshold distinguisher wins every time against kdf and is a coin flip against fieldKdf, whose pads cover [0, P). Reference vectors for fieldKdf and encryptField come from the same Python reference as the Fq and Sha256 specs. The mock drops the stale fieldKdfHalves. Refs: #735
This was referenced Sep 18, 2026
A pure mock ships no ZKIR, so the Sha256 rows could not be measured or reproduced. The mock now carries a counter like MockFq, and the specs reach it through a simulator. The IHasher contract type is declared without pure, since a pure declaration rejects such a mock while a pure export still satisfies the plain one. The @Constraints values are the ones measured through the mock.
Directory names use hyphens; the underscore came from the upstream Rust module path. Only the import strings and header lines change.
expandMessage is RFC 8017 MGF1 with mgfSeed = msg || DST and a 32-byte little-endian counter, not expand_message_xmd, whose strxor step has no Compact operator. The preimage struct and expandBlock use MGF1's names, C and counter, and the struct is exported. Every notation term links to its definition, the hash_to_field steps are spelled out, and the digest backends cite the ledger's persistent_hash and the ZKIR instruction.
kdf, pointDigest and fieldKdf state their formulas, fieldKdf cites the ECIES step it plays (SEC 1 §5.1.3) and the hash_to_field it is, and kdf says which SEC 1 KDF it is not. The kdf-versus-fieldKdf separation is stated on the preimages. The @Constraints values are re-measured through a ledger-backed mock; the previous ones did not come from one.
0xisk
changed the title
A pure mock ships no ZKIR, so the EcdhMask rows could not be measured or reproduced. The mock now carries a counter like MockFq and MockSha256, and the EcdhMask and token specs reach it through a simulator.
0xisk
marked this pull request as ready for review
September 18, 2026 12:36
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Depends on #866, merge after.
Types of changes
Closes #735
Closes #921
Closes #922
Closes #923
#735 asked for the 248-bit margin to be documented as a caveat. It is not a caveat:
ct = m + kdf(S)withkdf(S) < 2^248never wraps, so aFieldplaintext publishes its top bits, which is exactly the note-delivery case. So this adds a uniform mask,fieldKdf, as RFC 9380hash_to_fieldover a newcrypto/hashinterface and a newFqmodule, rather than a note onencrypt.Not visible in the diff:
kdfis unchanged on purpose. Its output is a published ciphertext component, and the golden vector pins it.fieldKdf's block preimages extendkdf's by a counter, so both may be used under one(S, domain).expand_message_xmd: the RFC's step 9 needsstrxor, and Compact has no XOR.hashlib,py_eccandECPy, kept with a runner against the compiled mocks at https://github.com/0xisk/midnight-crypto-verification (87 vectors). The pad-width leak is measured in the EcdhMask spec, withkdfand its absence withfieldKdf.@constraintsare reproducible. Every value was re-measured.Fq.compact, thenSha256.compact, thenfieldKdf.PR Checklist