Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Move Signer types out of the signature module #17099

Merged
merged 6 commits into from
May 11, 2021

Conversation

t-nelson
Copy link
Contributor

@t-nelson t-nelson commented May 7, 2021

Problem

Signer types cluttering the signature module.

Summary of Changes

Move them to their own modules (preserving legacy use-paths)

@t-nelson t-nelson requested a review from CriesofCarrots May 7, 2021 08:44
@t-nelson t-nelson changed the title Sdk breakup sig mod Move Signer types out of the signature module May 7, 2021
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

I like it!
Let's use this opportunity to add module- and struct-level doc comments? In particular, I think we should document: Signer trait, Presigner struct, Keypair struct, and prob keypair_from_seed(). I can follow with some or all of this, if you like.

sdk/src/signer/presigner.rs Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
@t-nelson
Copy link
Contributor Author

t-nelson commented May 8, 2021

I like it!
Let's use this opportunity to add module- and struct-level doc comments? In particular, I think we should document: Signer trait, Presigner struct, Keypair struct, and prob keypair_from_seed(). I can follow with some or all of this, if you like.

Sure! Seems like as good a time as any

@t-nelson t-nelson force-pushed the sdk-breakup-sig-mod branch 2 times, most recently from a23a008 to 18c7db7 Compare May 8, 2021 06:56
CriesofCarrots
CriesofCarrots previously approved these changes May 8, 2021
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Thanks for adding all the documentation! r+ nits and feature full

sdk/src/signature.rs Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
sdk/src/signer/mod.rs Outdated Show resolved Hide resolved
sdk/src/signer/mod.rs Outdated Show resolved Hide resolved
sdk/src/signer/keypair.rs Outdated Show resolved Hide resolved
sdk/src/signer/mod.rs Outdated Show resolved Hide resolved
@t-nelson t-nelson force-pushed the sdk-breakup-sig-mod branch from 18c7db7 to 6a9377d Compare May 11, 2021 06:07
@mergify mergify bot dismissed CriesofCarrots’s stale review May 11, 2021 06:08

Pull request has been modified.

@t-nelson t-nelson added automerge Merge this Pull Request automatically once CI passes v1.6 labels May 11, 2021
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label May 11, 2021
@mergify
Copy link
Contributor

mergify bot commented May 11, 2021

automerge label removed due to a CI failure

@t-nelson t-nelson force-pushed the sdk-breakup-sig-mod branch from 6a9377d to 88ed4a2 Compare May 11, 2021 17:11
@codecov
Copy link

codecov bot commented May 11, 2021

Codecov Report

Merging #17099 (88ed4a2) into master (d4ffd90) will decrease coverage by 0.0%.
The diff coverage is 83.2%.

@@            Coverage Diff            @@
##           master   #17099     +/-   ##
=========================================
- Coverage    82.7%    82.7%   -0.1%     
=========================================
  Files         417      421      +4     
  Lines      117718   117724      +6     
=========================================
- Hits        97416    97406     -10     
- Misses      20302    20318     +16     

@t-nelson t-nelson merged commit dbac387 into solana-labs:master May 11, 2021
@t-nelson t-nelson deleted the sdk-breakup-sig-mod branch May 11, 2021 19:08
}
/// Fallibly gets the implementor's public key
fn try_pubkey(&self) -> Result<Pubkey, SignerError>;
/// Invallibly produces an Ed25519 signature over the provided `message`
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I still see Invallibly here... crossed commits, maybe?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix in #17180

mergify bot added a commit that referenced this pull request May 11, 2021
…17177)

* sdk: Move `Signer` trait to own module

(cherry picked from commit af6f3d7)

* sdk: Move `Keypair` to `signer` module

(cherry picked from commit 0eba6eb)

* sdk: Move `Presigner` to `signer` module

(cherry picked from commit 12bf6c0)

* sdk: Move `NullSigner` to `signer` module

(cherry picked from commit b71e4bd)

* sdk: Move `signers` module into `signer` module

(cherry picked from commit 967840a)

* sdk: keypair - drop superfluous iter()

(cherry picked from commit dbac387)

Co-authored-by: Trent Nelson <trent@solana.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants