-
Notifications
You must be signed in to change notification settings - Fork 61
Validate decodePrivateKeyWif version and length #147
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
base: master
Are you sure you want to change the base?
Validate decodePrivateKeyWif version and length #147
Conversation
Signed-off-by: James Zuccon <zuccon@gmail.com>
|
| test('decodePrivateKeyWif: incorrect length (mainnet wif version, 20 bytes)', (t) => { | ||
| t.deepEqual( | ||
| // cspell: disable-next-line | ||
| decodePrivateKeyWif('tWGD2u9st6K9gUr68hdo53qhZZyk3JoQAF'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These WIFs have just been generated with the following code:
import { Base58AddressFormatVersion, encodeBase58AddressFormat, hexToBin, } from '@bitauth/libauth';
const mainnetWifInvalidLength = encodeBase58AddressFormat(Base58AddressFormatVersion.wif, new Uint8Array(20));
const testnetWifInvalidLength = encodeBase58AddressFormat(Base58AddressFormatVersion.wifTestnet, new Uint8Array(20));
console.log('mainnetWifInvalidLength', mainnetWifInvalidLength);
console.log('testnetWifInvalidLength', testnetWifInvalidLength);
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #147 +/- ##
=======================================
Coverage 98.35% 98.35%
=======================================
Files 152 152
Lines 61284 61296 +12
Branches 2272 2233 -39
=======================================
+ Hits 60276 60288 +12
Misses 1002 1002
Partials 6 6 ☔ View full report in Codecov by Sentry. |
e72ba49 to
60aec23
Compare
|
Hey @bitjson , is there anything else you'd like me to do for this one (e.g. can add the additional BTC/Segwit version codes if the general approach looks okay)? In terms of failed build, I think this might be something to do with the CI pulling a broken Node version. The code changes are pretty minimal and all passed locally. |
The
decodePrivateKeyWiffunction will currently allow a Legacy/Base58 BTC address.For example:
An example case where this might become pertinent is when a wallet scans a QR Code and has to distinguish between a WIF (for sweeping) and a Base58 Address (for sending funds to).
This PR does two things:
NOTE: I can't find a formalized spec for WIF and we might want to extend this PR a little bit.
The docs here: https://en.bitcoin.it/wiki/Wallet_import_format
... suggest that BTC might support some additional version codes:
it should be 0x80, however legacy Electrum or some SegWit vanity address generators may use 0x81-0x87... which we might want to add to
Base58AddressFormatVersionand also validate. Let me know if this is desired, will try to amend (unsure if we should still validate length in that case?).