Change context objects for verification methods

[sanket1729]

• The current schnorrsig verify methods should operate on verify context
  as is done throughout the bitcoin core
• Finally, and importantly the XonlyPublicKey::from_keypair now operates
  without any context parameter.

[dr-orlovsky]

Can we use preallocated context with all methods used by taproot tweaks? This will significantly simplify the API on the layers above for creating and working with taproot...

[real-or-random]

• The current schnorrsig verify methods should operate on verify context
  as is done throughout the bitcoin core

Yes. Even though in recent secp256k1 revisions, the verification context flag is ignored. It doesn't make it a difference, and it will likely be removed in the future.

* The API for convinience method is now slightly awkard as it requires
  both signing and verification contexts

Why should generate_schnorrsig_keypair need a verification context?

* Finally, and importantly the XonlyPublicKey::from_keypair now operates
  on Verification conetxt instead of signing. The type of context here
  does not matter so we can put any context here and it would not matter
  at all. But putting in Verification cleanly allows the verification
  routines to compute the XOnlyPublicKey from keypair that is required for
  computing the tweak in taproot.

Why should XOnlyPublicKey::from_keypair need a verification context in the first place?

[sanket1729]

Why should generate_schnorrsig_keypair need a verification context?

Unfortunately, all methods are restricted by a type system that requires context to be either Verification or Signing context. Any context object must be either 1) Signing, 2) Verification or 3) Both. Because it really does not matter, I suggest making it Verification because it really helps design cleaner code in rust-bitcoin. Another minor reason is that verify context also goes along with the notion of verifying taptweak.

Ideally, we want to provide a None context for these methods, but it is not currently supported in rust-secp, and supporting might not be worth it given everything would be static soon.

[real-or-random]

Ideally, we want to provide a None context for these methods, but it is not currently supported in rust-secp

Oh it is! We have ffi::secp256k1_context_no_precomp for this, see

rust-secp256k1/src/key.rs

Line 143 in 48683d8

ffi::secp256k1_context_no_precomp,

.

and supporting might not be worth it given everything would be static soon.

Not directly related to this PR but note that even with all precomputation being static, we'll still have (real) Signing contexts because they hold randomness for blinding secret key operations. So we wouldn't get rid of the context entirely for now, though we have the global_context feature.

edit:
generate_schnorrsig_keypair must require a Signing context because it does an EC mult with a secret key. (The term "Signing" is misleading here...) XOnlyPublicKey::from_keypair is the one that should use ffi::secp256k1_context_no_precomp. See #343 (comment) for more details.

[JeremyRubin]

concept ack just ran into this!

[JeremyRubin]

does signing context generate entropy on creation? testing now but this may be a big issue for using verification from a WASM module with no entropy because the get random number methods may panic.

[real-or-random]

does signing context generate entropy on creation? testing now but this may be a big issue for using verification from a WASM module with no entropy because the get random number methods may panic.

For manually generated contexts, this happens only if you randomize them explicitly, see

rust-secp256k1/src/lib.rs

Lines 354 to 380 in 48683d8

	/// (Re)randomizes the Secp256k1 context for cheap sidechannel resistance;
	/// see comment in libsecp256k1 commit d2275795f by Gregory Maxwell. Requires
	/// compilation with "rand" feature.
	#[cfg(any(test, feature = "rand"))]
	pub fn randomize<R: Rng + ?Sized>(&mut self, rng: &mut R) {
	let mut seed = [0u8; 32];
	rng.fill_bytes(&mut seed);
	self.seeded_randomize(&seed);
	}
	/// (Re)randomizes the Secp256k1 context for cheap sidechannel resistance given 32 bytes of
	/// cryptographically-secure random data;
	/// see comment in libsecp256k1 commit d2275795f by Gregory Maxwell.
	pub fn seeded_randomize(&mut self, seed: &[u8; 32]) {
	unsafe {
	let err = ffi::secp256k1_context_randomize(self.ctx, seed.as_c_ptr());
	// This function cannot fail; it has an error return for future-proofing.
	// We do not expose this error since it is impossible to hit, and we have
	// precedent for not exposing impossible errors (for example in
	// `PublicKey::from_secret_key` where it is impossible to create an invalid
	// secret key through the API.)
	// However, if this DOES fail, the result is potentially weaker side-channel
	// resistance, which is deadly and undetectable, so we take out the entire
	// thread to be on the safe side.
	assert_eq!(err, 1);
	}
	}

.

For the global context, it depends on the enabled feature: The global-context feature randomizes the context automatically and depends on rand, and the global-context-less-secure feature gives you a context that is not randomized (and can't be randomized).

[sanket1729]

Oh it is! We have ffi::secp256k1_context_no_precomp for this, see

I don't this is exposed at rust-secp level, only that at the sys level. See: https://docs.rs/secp256k1/0.20.3/secp256k1/trait.Context.html#implementors

Since the context does not matter (ideally we would want None, but it is unsupported), and we must have one of the implementors of the Context trait. It makes sense to have Verification for cleaner implementation downstream.

generate_schnorrsig_keypair must require a Signing context because it does an EC mult with a secret key.

Indeed, the current implementation has a Signing + Verification on this.

[real-or-random]

Oh it is! We have ffi::secp256k1_context_no_precomp for this, see

I don't this is exposed at rust-secp level, only that at the sys level. See: docs.rs/secp256k1/0.20.3/secp256k1/trait.Context.html#implementors

The idea is that you can make any rust-secp API function which does not need a special context take no context and under the hood use ffi::secp256k1_context_no_precomp.

Since the context does not matter (ideally we would want None, but it is unsupported),

Why is it not supported? You can just pass no context.

and we must have one of the implementors of the Context trait. It makes sense to have Verification for cleaner implementation downstream.

Why should it be cleaner to require a Context if none is needed?

Sorry, I may be terribly wrong and overlooking something but then please explain. :D I feel we're just talking past each other.

[elichai]

IMO converting a KeyPair to a Publickey is somewhat similiar to serializing a PublicKey, which uses ffi::secp256k1_context_no_precomp and doesn't require any context.

[apoelstra]

Ideally, we want to provide a None context for these methods, but it is not currently supported in rust-secp, and supporting might not be worth it given everything would be static soon.

We used to have a None. We removed it when we introduced the static none context many years ago. Why can't we use that here?

Edit: Oh, Tim brought this up already. What is the status of this PR then?

[sanket1729]

The PR has been updated so that

• Schnorr::PublicKey::from_keypair() does not have any context object
• schnorrsig verification methods take a verification context instead of a signing context.

[real-or-random]

I haven't looked at it yet but Concept ACK, this seems the right thing to do.

[apoelstra]

ACK 21aa914

The incorrect context on the verify_schnorr function seems like a pretty serious bug.