search

0xtrr / nostr-tool

A simple CLI to send nostr events
MIT License
60 stars 18 forks source link

Add vanity pubkey mining #21

Closed yukibtc closed 1 year ago

yukibtc commented 1 year ago
yukibtc commented 1 year ago

If changes are too many to review, I'll split this PR in multiples PRs.

0xtrr commented 1 year ago

Thanks for this PR @yukibtc! No that's ok, was only 4 commits.

The change re: error propagation, is this to propagate error messages from the nostr-sdk library instead of having/creating our own? Seems to clean up the code a bit which is always nice!

yukibtc commented 1 year ago

Thanks for this PR @yukibtc! No that's ok, was only 4 commits.

The change re: error propagation, is this to propagate error messages from the nostr-sdk library instead of having/creating our own? Seems to clean up the code a bit which is always nice!

It's to avoid to use .unwrap() that show errors with too much things. In this way the errors will be more cleaned: Error: InvalidChar('y') for example.

If I have time, I'll try to further improve display error messages.

0xtrr commented 1 year ago

Thanks for this PR @yukibtc! No that's ok, was only 4 commits. The change re: error propagation, is this to propagate error messages from the nostr-sdk library instead of having/creating our own? Seems to clean up the code a bit which is always nice!

It's to avoid to use .unwrap() that show errors with too much things. In this way the errors will be more cleaned: Error: InvalidChar('y') for example.

If I have time, I'll try to further improve display error messages.

As long as the user gets an error message that says what actually went wrong, it's all good to me. I'd say we can merge this now and make improvements later on if you want to

yukibtc commented 1 year ago

Yes, I'll open new PRs in the future :)

0xtrr commented 1 year ago

Yes, I'll open new PRs in the future :)

Allright, appreciate the PR and help with this project!