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

Remove AccountKeyring everywhere #5899

Merged

Conversation

programskillforverification
Copy link
Contributor

Close: #5858

@programskillforverification programskillforverification marked this pull request as ready for review October 22, 2024 06:29
@programskillforverification programskillforverification requested a review from a team as a code owner October 22, 2024 06:29
@paritytech-review-bot paritytech-review-bot bot requested a review from a team October 22, 2024 06:30
@shawntabrizi
Copy link
Member

shawntabrizi commented Nov 29, 2024

@programskillforverification can you please highlight with a comment the main place where AccountKeyring is defined and changed/removed?

Lots of files here :P

@@ -32,20 +32,11 @@ pub mod ed25519;
#[cfg(feature = "bandersnatch-experimental")]
pub mod bandersnatch;

/// Convenience export: Sr25519's Keyring is exposed as `AccountKeyring`, since it tends to be
/// used for accounts (although it may also be used by authorities).
pub use sr25519::Keyring as AccountKeyring;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shawntabrizi AccountKeyring definition here

#[cfg(feature = "bandersnatch-experimental")]
pub use bandersnatch::Keyring as BandersnatchKeyring;
pub use ed25519::Keyring as Ed25519Keyring;
pub use sr25519::Keyring as Sr25519Keyring;

pub mod test {
/// The keyring for use with accounts when using the test runtime.
pub use super::ed25519::Keyring as AccountKeyring;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shawntabrizi and here

Copy link
Member

Choose a reason for hiding this comment

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

doesn't seem this was used anywhere.

Copy link
Member

@shawntabrizi shawntabrizi left a comment

Choose a reason for hiding this comment

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

lgtm

@bkchr bkchr added the T17-primitives Changes to primitives that are not covered by any other label. label Nov 29, 2024
@bkchr bkchr requested a review from bkontur November 29, 2024 21:22
@bkchr bkchr enabled auto-merge November 29, 2024 21:23
auto-merge was automatically disabled November 29, 2024 22:41

Pull Request is not mergeable

@bkchr bkchr enabled auto-merge November 30, 2024 22:34
auto-merge was automatically disabled December 9, 2024 09:09

Head branch was pushed to by a user without write access

@bkchr bkchr enabled auto-merge December 9, 2024 22:04
@bkchr bkchr added this pull request to the merge queue Dec 10, 2024
Merged via the queue into paritytech:master with commit 311ea43 Dec 10, 2024
195 of 200 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T17-primitives Changes to primitives that are not covered by any other label.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove usage of AccountKeyring
4 participants