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

Swap 'base58' with 'bs58' #13739

Merged
merged 5 commits into from
Mar 29, 2023
Merged

Swap 'base58' with 'bs58' #13739

merged 5 commits into from
Mar 29, 2023

Conversation

davxy
Copy link
Member

@davxy davxy commented Mar 28, 2023 •
edited
Loading

Motivations:

  • base58 has hard limit of 128 bytes output (bs58 doesn't have any limit)
  • (marginal) bs58 is also used by libp2p so we depend on it anyway
  • (marginal) bs58 docs declares to be faster but I found the two almost equivalent.

@davxy davxy requested a review from a team March 28, 2023 16:44
@davxy davxy mentioned this pull request Mar 28, 2023
Merged
@davxy davxy self-assigned this Mar 28, 2023
@davxy davxy added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes 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 Mar 28, 2023
melekes
melekes approved these changes Mar 29, 2023
View reviewed changes
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍

@davxy davxy requested a review from a team March 29, 2023 07:59
koute
koute reviewed Mar 29, 2023
View reviewed changes
primitives/core/Cargo.toml Outdated Show resolved Hide resolved
bkchr
bkchr approved these changes Mar 29, 2023
View reviewed changes
primitives/core/Cargo.toml Outdated Show resolved Hide resolved
@davxy davxy merged commit 49ba186 into master Mar 29, 2023
@davxy davxy deleted the davxy-introduce-bs58 branch March 29, 2023 13:46
@nazar-pc nazar-pc mentioned this pull request Apr 2, 2023
Merged
1 task
pgherveou pushed a commit that referenced this pull request Apr 4, 2023
@davxy @pgherveou
Swap 'base58' with 'bs58' (#13739)
7bba9d2 
* Swap base58 with bs58

* Removed unused clone

* std flag
gpestana pushed a commit that referenced this pull request Apr 23, 2023
@davxy @gpestana
Swap 'base58' with 'bs58' (#13739)
2482f20 
* Swap base58 with bs58

* Removed unused clone

* std flag
@jasl jasl mentioned this pull request May 11, 2023
Merged
@kacperzuk-neti kacperzuk-neti mentioned this pull request Jun 23, 2023
Merged
15 tasks
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
@davxy @nathanwhit
Swap 'base58' with 'bs58' (paritytech#13739)
238ec35 
* Swap base58 with bs58

* Removed unused clone

* std flag
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr approved these changes

@koute koute koute approved these changes

@melekes melekes melekes approved these changes

@michalkucharczyk michalkucharczyk michalkucharczyk approved these changes

Assignees

@davxy davxy

Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@davxy @koute @melekes @michalkucharczyk @bkchr