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

Conversation

@Dylan-DPC-zz
Copy link

Closes #213

Added Base58 as a dependency to this sub-crate
Since Base58 only accepts a [u8] or str, added a trait that needs the type to be implemented on to be convertable to a vector.

Copy link
Contributor

@rphmeier rphmeier left a comment

Choose a reason for hiding this comment

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

A couple grumbles:

  • SS58 is a bit different than regular base58. Check out the ed25519 crate and the to_ss58check method.
  • Substrate runtime components should bound simply by Display, and the implementations should do the ss58check internally. This might require a PR in https://github.com/paritytech/polkadot now that we've split the projects.
  • Tabs, not spaces.
  • Formatting: a lot of extraneous spaces in the code

@Dylan-DPC-zz Dylan-DPC-zz changed the title Display for Address/AccountId/AccountIndex [WIP] Display for Address/AccountId/AccountIndex Sep 3, 2018
@Dylan-DPC-zz
Copy link
Author

Notes:

  • Changed to ss58
  • I had to keep those trait bounds as I need to ensure that Index & Id can be converted into a SS58.
  • Having an issue adding the trait bounds to the macros so working on that
  • Will do the formatitng at the end

@gavofyork gavofyork added A1-onice A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. and removed A1-onice labels Sep 24, 2018
@gavofyork
Copy link
Member

pretty stale now - if interesting in continuing on this please reopen for guidance.

@gavofyork gavofyork closed this Sep 29, 2018
lamafab pushed a commit to lamafab/substrate that referenced this pull request Jun 16, 2020
…ritytech#598)

* Update Substrate after repository reorganisation

* Switch back to polkadot-master

* Bump `bitvec` and `parity-scale-codec` (paritytech#591)

Also bump other dependencies, but respect semver on them.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants