[frost] backup scheme

[nickfarrow]

Explanation and justification of scheme in module documentation.

This draft scheme currently includes the ability for the share index to either be a small integer or a scalar.

• draft scheme
• figure out what is going on with checksum length and warn about error-correction if necessary.
• encode to bech32m and decode
• check ive implemented optional share_backup feature and bech32 dependency correctly
• restructure FROST internally to use polynomials
• more comprehensive tests

frostsnap-cli backup and restore (separate PR)

[LLFourn]

1. Given a vector of shares we should have a function that tries to reconstruct the main secret.
2. Given a vector of share images should try and reconstruct the public polynomial and therefore give us the ability to check the id in the encoding.

[nickfarrow]

I've also taken this as a good time to create a separate module for polynomials secp256kfun::poly.
By including all the lagrange interpolation utilities in here, the secret share backup scheme does not depend on frost.rs.

I made the FrostKey store the public polynomial internally, and use it to generate verification shares and threshold.

Two things of interest:

We lose sight of which signer indexes exist internally, and also pub fn n_signers(&self) -> usize. We don't use it anywhere inside our logic, so does this matter? It might mean that users of this library will need to store signers & number of signers separately alongside the FrostKey. This kind of makes sense as we abstract away from a FrostKey having a fixed number of signers.

Secondly, I am wondering if we want to go further and remove storing the FROST public_key as we are currently? Since it changes with tweaks depending on the FrostKey point-type, it strays from point_polynomial[0]. Or maybe we just rename it to tweaked_public_key.

Perhaps we could do something like accumulate these tweaks internally until .public_key() is called. I recall we didn't like the API of another implementation which took vecs of ordinary & xonly tweaks as arguments. Do you have any thoughts on this or our current FrostKey<T> API?

[LLFourn]

I've also taken this as a good time to create a separate module for polynomials secp256kfun::poly. By including all the lagrange interpolation utilities in here, the secret share backup scheme does not depend on frost.rs.

Cool.

I made the FrostKey store the public polynomial internally, and use it to generate verification shares and threshold.

Two things of interest:

We lose sight of which signer indexes exist internally, and also pub fn n_signers(&self) -> usize. We don't use it anywhere inside our logic, so does this matter? It might mean that users of this library will need to store signers & number of signers separately alongside the FrostKey. This kind of makes sense as we abstract away from a FrostKey having a fixed number of signers.

Yes this is the way to go.

Secondly, I am wondering if we want to go further and remove storing the FROST public_key as we are currently? Since it changes with tweaks depending on the FrostKey point-type, it strays from point_polynomial[0]. Or maybe we just rename it to tweaked_public_key.

We shouldn't store on disk but we should have a separate public key field in memory. tweaked_public_key SGTM.

Perhaps we could do something like accumulate these tweaks internally until .public_key() is called. I recall we didn't like the API of another implementation which took vecs of ordinary & xonly tweaks as arguments. Do you have any thoughts on this or our current FrostKey<T> API?

I think the current API is good.

[LLFourn]

I think we have the / operator for division now if you want to use it.

[LLFourn]

This was resolved but you didn't use it? Maybe it's because you can't call invert on non-zero thingies. I didn't think of that. I like that it was made explicit then!

[nickfarrow]

I didn't use it here because simplest form in the product was $(a_m * x + b_m)$ terms rather than $(x / a_m + b_m)$, also yeah to use / here you'd have to do something like

let a_m_denom = s!(x_j - x_m)
    .non_zero()
    .expect("points must lie at unique indicies to interpolate");
let a_m = s!(1 / a_m_denom);

[nickfarrow]

Added some of the suggested functions poly::add and poly::point_mul, makes the lagrange interpolation code more understandable.

Changed threshold to take two characters (max threshold 1024).

Added a panic to encode if the secret share is not valid with respect to the point polynomial.

---

I'd like to squash this mess of commits before merging (something like: 1 commit for poly and 1 commit for backup scheme)

[nickfarrow]

One thing I haven't implemented here is having an EncodableFrostKey that doesnt serialize the tweaked public key

[nickfarrow]

Added ShareBackup struct, I think this API is nice!

There's a duplicated threshold < 1024 because of the floating encode but i think this is fine.

Reminder that I'd like to squash these commits after any approval.
I think I like this flow of keeping working commits separate for easy review, and then squashing appropriately so our master looks perfect.

[LLFourn]

Left every nit I can find. Looks really really god.

One thing I haven't implemented here is having an EncodableFrostKey that doesnt serialize the tweaked public key

Please do this and we'll be done here. It should be called EncodedFrostKey.

[nickfarrow]

Please check what i've done with EncodedFrostKey, is this what you had in mind? It only stores the point polynomial, from which an untweaked FrostKey can be restored. I guess we don't even want to store the Secret Share here because we'll be using better security for that.

I've removed the serde and bincode stuff from FrostKey

[nickfarrow]

I was just exploring a test that randomly deletes n random characters from the backup. It doesn't seem the bech32 is doing the error correction we would like it to, I just get Bech32DecodeError(bech32::Error::InvalidChecksum) even when deleting a single character.

Sure the checksum is invalid, but i'd think for bech32 to be useful we want to see the corrected decoding suggestion(s)? InvalidChecksum(PotentialDecoding)

Edit: It's on the stabilization roadmap rust-bitcoin/rust-bech32#95

[LLFourn]

LGTM. Thanks.

I fixed up one thing. The decode/deserialize impls for EncodedFrostKey should not allow zero as the first coefficient.

[LLFourn]

This should have custom decode and deserialize implementations that just try and decode to Vec<Point<..,Zero>> and check that the first coef is not zero.

[LLFourn]

(update: I did this)