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

Fix user import_wallet #20

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Fix user import_wallet #20

merged 1 commit into from
Apr 30, 2024

Conversation

alex-stone
Copy link
Contributor

@alex-stone alex-stone commented Apr 30, 2024

What changed? Why?

This fixes the user #import_wallet reference for wallets_api to ensure that it uses the memoized helper.

Currently this is returning an error since @wallets_api is not set unless you initialize a wallet with address_count: 0 or calling list_balances.

This can be verified with:

user = Coinbase.default_user
cb_wallet = user.import_wallet(Coinbase::Wallet::Data.new(wallet.seed, wallet.wallet_id))

Qualified Impact

This impacts wallet import flows

This fixes the user import_wallet to ensure that it uses the memoized
wallets_api helper.

This currently is returning an error when importing a wallet in
the following manner:

```ruby
user = Coinbase.default_user
cb_wallet = user.import_wallet(Coinbase::Wallet::Data.new(wallet.seed, wallet.wallet_id))
```
@yuga-cb yuga-cb merged commit 458f5b8 into v0.0.2 Apr 30, 2024
3 of 4 checks passed
@yuga-cb yuga-cb deleted the stone/fix-import-wallet branch April 30, 2024 20:27
alex-stone added a commit that referenced this pull request Apr 30, 2024
This adds a regression test for this PR:
#20
alex-stone added a commit that referenced this pull request Apr 30, 2024
This adds a regression test for this PR:
#20
alex-stone added a commit that referenced this pull request Apr 30, 2024
### What changed? Why?
This makes the Coinbase::Wallet::Data object keyword initialized.

Since users have to persist this data they will need to rehydrate this.
Having the ordering matter makes it more likely for developers to= run
into errors (as I did when integrating the SDK).

This also adds a regression test for this PR:
#20

#### Qualified Impact
<!-- Please evaluate what components could be affected and what the
impact would be if there was an
error. How would this error be resolved, e.g. rollback a deploy, push a
new fix, disable a feature
flag, etc... -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants