Skip to content
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

Secure Signing e2e Integration #993

Open
wants to merge 116 commits into
base: main
Choose a base branch
from

Conversation

l-monninger
Copy link
Collaborator

Summary

  • RFCs: \emptyset$.
  • Categories: misc.

Integrates secure signing work and prepares e2e testing.

  • To facilitate e2e testing, a common Identifier is used to render a LoadedSigner struct, which currently wraps any of the HashiCropVault, AwsKms, or Local signers provided here.
  • An example usage of loading the a Secp256k1 signer from the Config into the DA Light Node is implemented here.
  • Required next steps are the successful updates and replacements of MAPTOS_PRIVATE_KEY, MCR_SETTLEMENT_KEY, and MOVEMENT_DA_SIGNER to match the canonical string serialization here, s.t. the overlay runs and can toggel different backends. The overlay will then be expanded upon.

Changelog

Testing

Outstanding issues

l-monninger and others added 30 commits December 17, 2024 10:38
Provide From conversion impls to construct
PrivateKey and Signature from byte arrays.
The conversions into PrivateKey are fallible, while
a Signature is allowed to be constructed from any
64 bytes.
Add TestSigner providing an in-process Signing
implementation for tests.
TryFromBytes is not needed, neither is the public key
as a member of HashiCorpVault state.
Comment on lines 37 to 40
pub verifier: Arc<
Box<dyn VerifierOperations<CelestiaBlob, IntermediateBlobRepresentation> + Send + Sync>,
>,
pub signing_key: SigningKey<C>,
pub signer: Arc<Signer<O, C>>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're punting the blob signature curve choice as a generic parameter up to here, but the VerifierOperations trait does not fix it, so...


Ok(Self {
config: config.clone(),
celestia_namespace: config.celestia_namespace(),
default_client: client.clone(),
verifier: Arc::new(Box::new(Verifier::<C>::new(
verifier: Arc::new(Box::new(Verifier::<Secp256k1>::new(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... so this concrete implementation is responsible for hooking the verifier with the correct curve choice.

@mzabaluev
Copy link
Collaborator

The sec256k1::send_tx test fails.

Co-authored-by: Richard Melkonian <[email protected]>
Co-authored-by: Icarus131 <[email protected]>
Co-authored-by: primata <[email protected]>
Co-authored-by: musitdev <[email protected]>
Co-authored-by: Andy Golay <[email protected]>
Co-authored-by: Icarus131 <[email protected]>
Co-authored-by: Mikhail Zabaluev <[email protected]>
@andygolay
Copy link
Contributor

The sec256k1::send_tx test fails.

I saw I was pinged for a review.

What is the command for running this and the other tests? I see the tests in util/signing/testing/tests but haven't managed to run them yet with various attempted commands.

Also does util/signing/testing needed to be added to the workspace.members array in the root Cargo.toml? noticed it's not in there. I got an error about it with one test command I tried.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants