validate the contents of seed before creating a Wallet object

ckeshava commented 7 months ago

High Level Overview of Change

Fixes https://github.com/XRPLF/xrpl-py/issues/503

It's ideal to validate a seed before creating a Wallet object. This is helpful in generating readable error messages for the users.

Context of Change

Type of Change

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Refactor (non-breaking change that only restructures code)
[x] Tests (You added tests for code that already exists, or your new feature included in this PR)
[ ] Documentation Updates
[ ] Release

Did you update CHANGELOG.md?

[ ] Yes
[x] No, this change does not impact library users Invalid seed input to Wallet will throw a different error. The current code throws a ValueError, this is hard to understand for users.

Test Plan

I have added a unit test that validates the Exception thrown upon empty seed inputs into the Wallet class constructor.

ckeshava commented 7 months ago

i don't understand why the seed is an Optional[str] in Wallet's constructor here: https://github.com/XRPLF/xrpl-py/blob/a38f61d3c88b4a9a0807392ef4b0cb30aa688ce5/xrpl/wallet/main.py#L45

In what kind of situations can we get away with a non-existent seed? I saw a unit test that does not specify a seed for the Wallet, but I couldn't find any other examples. https://github.com/XRPLF/xrpl-py/blob/a38f61d3c88b4a9a0807392ef4b0cb30aa688ce5/tests/unit/asyn/wallet/test_main.py#L81

JST5000 commented 7 months ago

i don't understand why the seed is an Optional[str] in Wallet's constructor here:

You can generate a wallet if you know the public / private key.

Also I would mark this as a change for end users and update HISTORY.md (changing error messages counts because they may have error handling for a specific error type - not changing HISTORY.md is almost exclusively for docs / testing changes or functionally equivalent refactors).

ckeshava commented 7 months ago

I see. So a seed is used to generate a public/private pair of keys. But I can bypass that step if I already know the cryptographic key pair.

ckeshava commented 6 months ago

@justinr1234 can you please review this PR at your convenience?

XRPLF / xrpl-py