-
Notifications
You must be signed in to change notification settings - Fork 18
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
Update to wsts 12 #1265
base: main
Are you sure you want to change the base?
Update to wsts 12 #1265
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sBTC side:
- We can remove
SignerStateWrapper
andPartyStateWrapper
now that they implement eq (in https://github.com/stacks-network/sbtc/blob/main/signer/src/proto/convert.rs#L1802-L1834) and move the tests inconvert_protobuf_type3
toconvert_protobuf_type2
WSTS side:
- in
check_public_shares
: maybe switch the order of the checks (first len, then verify)? I'm not sure ifverify
does something bad if the poly len is wrong make_shared_secret_from_key
(callingansi_x963_derive_key
) generates a different key now because of the counter and the order of hash updates, is it fine (wrt backward compatibility with data already generated)? Does this makes this a breaking protocol change (ie, if signers are running different versions can they still interact as expected)?- maybe nit:
fn gen<RNG: RngCore + CryptoRng>(rng: &mut RNG) -> Scalar
: as this don’t use a secret, doing the generation as in 4.1 (see comment) doesn’t seem to give any better guarantees: if the rng is a bad one, this will still generate bad nonces
Will do.
Good point; this would definitely break if one of the signers was using code with a different key derivation. And a signer using the new key derivation would not be able to read old DkgPrivateShares, though we aren't persisting them past DKG so that shouldn't be an issue.
Interesting, I read this as basically saying to hash two 32-bytes random values rather than just using a single 32 byte value directly. But yeah if the RNG is predictable then you really aren't gaining much. Looking over the rest of the doc, it looks like "secret key" is used mostly to mean the key shares, but in some places it isn't really defined at all. We could use the key shares, or pass in the ECDSA key. What do you think? |
Just to double check: we store (encrypted) the whole
The bit that got me thinking was:
So if we don't have something secret, the security of it should be the same as using directly the RNG without the extra manipulation. Given how it is used in |
DkgPrivateShares are not stored in those state objects. The polynomials and key shares (which are called private keys) are.
Agreed. |
Description
Update the
wsts
dependency to12.0.0
Closes #1162, closes #1166, and closes #1167
Changes
Testing Information
Checklist: