refactor(crypto): expose BLS as free functions - #664
refactor(crypto): expose BLS as free functions#664emlautarom1-agent[bot] wants to merge 5 commits into
Conversation
`pluto_crypto` exposes BLS operations as plain `pub fn`s in `tbls`, matching the style `pluto-k1util` already uses. Call sites read `tbls::verify(...)` instead of materializing a zero-sized receiver and importing a trait. The blst scalar and curve-point arithmetic moves to a private `tbls::math`, which holds every `unsafe` block in the crate. The crate lint is `unsafe_code = "deny"` so that adding `unsafe` anywhere else fails the build, and `math` opts back in locally. It is `deny` rather than the workspace `forbid` because `forbid` cannot be re-enabled for `math`; a local `#[allow(unsafe_code)]` can therefore still defeat it. Closes #602
The key and signature conversions live next to the types they operate on, in `pluto_crypto::types`, rather than in a module of their own. Call sites read `types::pubkey_from_bytes(...)`, and `tbls` is left holding only the BLS operations.
`signeddata` calls `pluto_crypto::types::sig_to_eth2` instead of carrying its own copy. `pluto_core` already depends on `pluto-crypto` and re-exports `Signature` from it, so the two were the same identity conversion over the same `[u8; 96]`.
`Error::BlsError` and the `From<BLST_ERROR> for Error` impl that was its only constructor are gone: no fallible call returns `BLST_ERROR`, so the variant could never be observed. Every real blst failure is already mapped to a specific variant carrying a `BlsError`. `InvalidSecretKey` also loses its `#[from]`. Nothing returns `Result<_, BlsError>`, so the conversion was never exercised, and a future `?` on a public-key or signature parse would have been labelled a secret-key failure. It now matches its three sibling variants, which carry a `BlsError` without a blanket conversion.
`evaluate_polynomial` updates `x_power` unconditionally and hoists the `x` scalar out of the loop. The guard it replaces was expressed as a property of the polynomial's degree rather than of the loop position, so it read like a bounds bug when it was only avoiding one multiply on a degree-1 polynomial. That multiply now happens and its result is discarded; the value returned is unchanged.
![emlautarom1-agent[bot]](https://avatars.githubusercontent.com/u/31224949?s=60&v=4){kind=small}
| // Horner-free evaluation: `x_power` holds x^i entering iteration i. | ||
| let x_scalar = scalar_from_u64(x); | ||
| let mut x_power = x_scalar.clone(); | ||
|
|
||
| for coeff in poly.iter().skip(1) { | ||
| // result += coeff * x_power | ||
| let term = scalar_mult_secret(coeff, &x_power)?; | ||
| result = scalar_add_secret(&result, &term)?; | ||
|
|
||
| x_power = scalar_mult_scalars(&x_power, &x_scalar)?; |
There was a problem hiding this comment.
Only arithmetic change in the branch. The replaced guard (if poly.len() > 2) was loop-invariant, so for len >= 3 the old code already ran the trailing discarded multiply. The sole delta is threshold == 2, which now does one extra multiply whose result is never read.
It cannot fail: blst_sk_mul_n_check fails only when the product is zero mod r, and both operands are x, drawn from 1..=total with total <= 255.
| /// BLST error. | ||
| #[error("BLST error: {0}")] | ||
| BlsError(BlsError), | ||
| InvalidSecretKey(BlsError), |
There was a problem hiding this comment.
Dropping #[from] also drops source(), since thiserror's #[from] implies #[source]. Display is unchanged.
The conversion was never exercised — nothing returns Result<_, BlsError> — but it was a footgun: a future ? on a public-key or signature parse would have been labelled a secret-key failure. The three error-chain consumers were checked; none is reachable with this variant.
#[source] in place of #[from] would keep the chain, if that beats sibling consistency.
| unsafe_code = "allow" | ||
| # `deny` rather than the workspace `forbid` so `tbls::math` — the sole module | ||
| # wrapping the blst C bindings — can opt back in with `#![allow(unsafe_code)]`. | ||
| unsafe_code = "deny" |
There was a problem hiding this comment.
deny, not forbid All unsafe now lives in the private tbls::math, which opts back in with #![allow(unsafe_code)], impossible under forbid.
emlautarom1
left a comment
emlautarom1
left a comment
There was a problem hiding this comment.
Checked manually, mostly mechanical changes due to dropping BlstImpl
The single simplification in the math module has been verified to be correct.
1 participant
Closes #602
Summary
pluto_cryptoexposed BLS through aTblstrait with one implementor, the zero-sizedBlstImpl, so every call site had to materialize the ZST and import the trait. The trait was never used polymorphically. BLS is now plainpub fns, matchingpluto-k1util.This moves toward Charon, not away from it. Charon's
tbls.Implementationexists to backSetImplementation, called from exactly one place in the Go tree — its own test file. Charon's callers only ever see the package-leveltbls.Sign/tbls.Verify. Pluto never had that swap point.Commits
Each is independently buildable; the branch bisects cleanly.
tbls::mathand tightens the crate lint tounsafe_code = "deny", with that module opting back in.types— foldstblsconvin next to the types it converts.tblsno longer depends onpluto-eth2api.sig_to_eth2helper —pluto_core::signeddatacarried its own copy of the same identity conversion over the same[u8; 96].Errorvariant whose only constructor was never invoked, and a#[from]never exercised. See inline note.