search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.86k stars 680 forks source link

[Request] Move `substrate-bip39` into `polkadot-sdk` monorepo #1934

Closed kayabaNerve closed 8 months ago

kayabaNerve commented 1 year ago

https://github.com/paritytech/substrate-bip39/issues/17, yet placed here. I'd note the lack of eyes on the prior issue demonstrates the reasoning for why.

kayabaNerve commented 12 months ago

substrate-bip39 does not implement BIP-39 (it runs pbkdf2 on the entropy, not the mnemonic). If one is willing to make that breaking change, the one function polkadot-sdk/substrate/ uses from it has an equivalent in tiny-bip39, enabling removing the substrate-bip dependency.

ggwpez commented 12 months ago

I a valid reason to move it to the SDK is that it is a substrate-specific (the opposite of generally applicable) crate?
Looks like the crate just contains two functions, seems inded a bit overkill for its own repo.

kayabaNerve commented 12 months ago

Considering that leads it to be ignored, despite being a dependency for effectively the entire Substrate ecosystem, I'd say so. There's no argument I can think of for why it should have an isolated issue board as it has no distinct scope nor such a large scope it'd be obtrusive to be here.

Though if the breaking change isn't unacceptable, I'd call for it to be dropped.

ggwpez commented 12 months ago

It makes sense to me, but this is normally not decided by a single person. Maybe you can open a merge request for it? Should be pretty small.

bkchr commented 12 months ago

We also don't have SCALE in the repo... frame metadata is also living in its own repo. There is no real need to have everything "substrate" in this one repo.

bkchr commented 12 months ago

And just because it moves here doesn't mean that it gets more attention..

kayabaNerve commented 12 months ago

SCALE has been widely adopted outside of Substrate and has a general purpose use case. I'd rather see it argued SCALE should drop from its README its used in Substrate and insert it's an efficient codec notably used in Substrate than argue for its inclusion in this repo.

As for increased attention, it'll become subject to automated tooling present here, any dependency CTRL+Fs, and since it'd be in workspace can take advantage of workspace = true if/when adopted.

@ggwpez I would've made the PR instead of an issue to prompt the PR except I'm reasonably unable to sign the CLA, sorry :/ I'm unsure if that'd be an issue raised if I had make the PR, as while the automated system would require it, it's clear to any human no actual independent contribution was made.