Skip to content
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

lib: added key string constants, chainparams and bip32/44 wrappers #158

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

edtubbs
Copy link
Collaborator

@edtubbs edtubbs commented Sep 16, 2023

address: added depth to dogecoin_hd_generate_key for extended keys
chainparams: added isTestnetFromB58Prefix and isMainnetFromB58Prefix wrappers
bip32: added wrappers and depth to dogecoin_hd_generate_key for extended keys
bip39: moved constant to header
bip44: added wrappers and updated to derive_bip44_extended_key
key: added wrappers
dogecoin: added key string and script constants
example: moved constants to header, updated for key string constants, indentation and to derive_bip44_extended_key
tool: added wrappers, assert for dogecoin_pubkey_get_hex call and depth to dogecoin_hd_generate_key for extended keys
tx: added wrapper
vector: added wrappers for serialization/deserialization
test: added wrappers to bip32, key, transaction, tool, and vector tests

address: added depth to dogecoin_hd_generate_key for extended keys
chainparams: added isTestnetFromB58Prefix and isMainnetFromB58Prefix wrappers
bip32: added wrappers and depth to dogecoin_hd_generate_key for extended keys
bip39: moved constant to header
bip44: added wrappers and updated to derive_bip44_extended_key
key: added wrappers
dogecoin: added key string and script constants
example: moved constants to header, updated for key string constants, indentation and to derive_bip44_extended_key
tool: added wrappers, assert for dogecoin_pubkey_get_hex call and depth to dogecoin_hd_generate_key for extended keys
tx: added wrapper
vector: added wrappers for serialization/deserialization
test: added wrappers to bip32, key, transaction, tool, and vector tests
@edtubbs
Copy link
Collaborator Author

edtubbs commented Sep 16, 2023

Tested on x86 windows, linux and aarch64 linux.

@edtubbs edtubbs force-pushed the 0.1.3-dev-wrappers branch 4 times, most recently from 1e0a334 to e2f8666 Compare September 19, 2023 00:57
@edtubbs
Copy link
Collaborator Author

edtubbs commented Sep 19, 2023

Retested examples on x86 windows, linux and aarch64 linux.

@edtubbs
Copy link
Collaborator Author

edtubbs commented Sep 19, 2023

Retested example with correct result.

test: added print of extended key to tool_tests

doc: added extended public key derivation
@edtubbs
Copy link
Collaborator Author

edtubbs commented Sep 19, 2023

Retested with update to tool_test.

@xanimo
Copy link
Member

xanimo commented Sep 19, 2023

one thing i just noticed with regards to the tool test is there are no guards per hardening within hd_derive. i'm wondering if this should return an error prompt if a user passes ' or h? for instance, adding a ' or h:

    size_t extoutsize = 200;
    char* extout=dogecoin_char_vla(extoutsize);
    const char* privkey = "dgpv557t1z21sLCnAz3cJPW5DiVErXdAi7iWpSJwBBaeN87umwje8LuTKREPTYPTNGXGnB3oNd2z6RmFFDU99WKbiRDJKKXfHxf48puZibauJYB";
    debug_print("\nMaster private key:  %s\n", privkey);

    u_assert_int_eq(hd_derive(&dogecoin_chainparams_main, privkey, "m/0h", extout, extoutsize), true);
    // u_assert_str_eq(extout, "dgpv55wVA8mg2HkLPXa4bSyi83hbXrwVWsiTE2Z3kSTUtC6QUyg3ed3FprcvAFKWUSvyRtaPuVwfbcQMQqVXM12yrxQtSCB2iPF4A6RdDp53jjy");
    printNode(false, extout);

yields an overflow here:

ext key:             dgpv55wVA8mpMxHJbEck3FctD5WKCUMRHgDqmcDLkVodsuN4846ZmSFRpuvWjg1E4w1RwkxxRHJPgpspN64AHaWzcQU5QQDjBTFZHCA9ttGNEBT
extended pubkey:     dgub8ppWVphhv6wdt6Gu5VErKw3uf2j47oFZ4CzYucCcg2R4g7DGBCoCUDLAJvJSAutDfVETaiWDaGJvgEDKawWRCkBAU2MTmZ8K83JNaBoFbhb
pubkey hex:          03888018e1c71a8fa3be86f3e5536c7dfa5916a983ef2c7c3f1076d751125ec571
privatekey WIF:      QVjay8K1ktSjCEoHRq3oftCRT1pqtGDgWuGe3AF8ZAAwaAAbnpKJ
depth:               2
child index:         -2147483648
p2pkh address:       DNMeGu5X8ptm9hFETMEswHbV8QjnAFKBRe

this isn't a blocker per this pr but just wanted to note.

@michilumin michilumin self-requested a review September 28, 2023 05:08
Copy link
Member

@xanimo xanimo left a comment

Choose a reason for hiding this comment

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

ACK, built and tested on x86_64-pc-linux-gnu.

Copy link
Contributor

@michilumin michilumin left a comment

Choose a reason for hiding this comment

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

big PR but much of that i know is whitespace/tab deletion, lgtm though.

@edtubbs edtubbs merged commit fca46f8 into dogecoinfoundation:0.1.3-dev Oct 3, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants