Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Switch to using ss58-registry crate #9755

Merged
merged 33 commits into from
Oct 12, 2021

Conversation

gilescope
Copy link
Contributor

@gilescope gilescope commented Sep 11, 2021

Move ss58 json and enum definition into its own crate. Fixes #9696

Changes:
Default using atomic rather than a mutex.
u8/u16 conversions are From rather than TryFrom as they are infallible due to the Custom(u16) arm of the enum.
O(ln(n)) rather than O(n) perf for lookup methods.
No way to have a custom representation and a known representation that seem unequal but are eq.
Memory size is now the same size as u16 (was twice that).

(See also paritytech/ss58-registry#8 and companion: paritytech/polkadot#3953 )

@gilescope gilescope added A3-in_progress Pull request is in progress. No review needed at this stage. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Sep 11, 2021
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
@gilescope
Copy link
Contributor Author

gilescope commented Sep 13, 2021

(Added composable finance to the ss58 crate)

@gilescope gilescope added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Sep 13, 2021
primitives/core/Cargo.toml Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
@gilescope
Copy link
Contributor Author

Ah ok. Will have to be two crates then - an ss58-registry crate with just the enum in and a procmacro crate called ss58-registry-derive that defines the macro.

@gilescope
Copy link
Contributor Author

Tempted to rename KnownSs58AddressFormat to KnownNetwork

@gilescope gilescope changed the title WIP - Switch to using ss58-registry crate Switch to using ss58-registry crate Sep 16, 2021
primitives/core/Cargo.toml Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
@gilescope
Copy link
Contributor Author

finally. @bkchr I think the only thing it needs now is an approval on the companion PR.

@gilescope gilescope requested a review from bkchr October 10, 2021 20:23
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
gilescope and others added 4 commits October 11, 2021 13:31
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@bkchr bkchr requested a review from andresilva October 11, 2021 12:40
primitives/core/src/crypto.rs Outdated Show resolved Hide resolved
Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
@gilescope
Copy link
Contributor Author

Will merge this in tomorrow morning.

@gilescope gilescope merged commit cafe12e into paritytech:master Oct 12, 2021
@nazar-pc
Copy link
Contributor

Shouldn't ss58-registry.json file be removed as well?

@thiolliere
Copy link
Contributor

Shouldn't ss58-registry.json file be removed as well?

done in #10094

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move ss58-registry into its own crate
6 participants