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

feat: add public wallet #561

Merged
merged 12 commits into from
Oct 31, 2022
Merged

feat: add public wallet #561

merged 12 commits into from
Oct 31, 2022

Conversation

luizstacio
Copy link
Member

@luizstacio luizstacio commented Oct 28, 2022

This PR split the Wallet functions into two different levels;

  • WalletLocked: Can be initialized just with an address. This enables the account to do public queries, execute dry-run transactions and send transactions directly to the provider without signing features.
  • WalletUnlocked: Implements WalletLocked, but overrides send transaction and simulates for signing transactions. It also implements sign messages.
  • Convert the Wallet class to a utility class with all strategies to generate a WalletLocked or WalletUnlocked.

The main difference is that WalletUnlocked has access to the PrivateKey, and WalletPublic works only with the Address.

Also, I add a new arrayify function. The idea of the new arrayify is to enable correctly parsing an object without a length. This is needed in order to stringify a transaction and be able to recreate a new TransactionRequest object from the parse of the string.

I add a new test on contract.test.ts to test the stringify and a test for emulating a custom provider using a WalletPublic.

These changes are enabling fuels-ts integration with custom provider objects, like Wallet Extension now and Hardware Wallets in the future.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 28, 2022

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
90.14% (+0.85% 🔼)
3556/3945
🟡 Branches
72.52% (+1.78% 🔼)
694/957
🟢 Functions
87.99% (+1.34% 🔼)
718/816
🟢 Lines
90.15% (+0.92% 🔼)
3406/3778
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / base-locked-wallet.ts
91.86% 70% 93.33% 91.86%
🟢
... / constants.ts
100% 100% 100% 100%
🟢
... / base-unlocked-wallet.ts
100% 100% 100% 100%
🟢
... / wallets.ts
95.45% 75% 85.71% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟡
... / provider.ts
72.19% (-2.47% 🔻)
57.14%
73.17% (-1.83% 🔻)
70.42% (-2.57% 🔻)

Test suite run success

541 tests passing in 49 suites.

Report generated by 🧪jest coverage report action from 852874c

@luizstacio luizstacio marked this pull request as draft October 29, 2022 18:39
Copy link
Contributor

@camsjams camsjams left a comment

Choose a reason for hiding this comment

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

I think it looks good, but want to discuss names more.

I feel like the primary name should be Wallet and then we'd name the signed version with private key using a different name.

Like:
WalletPublic => Wallet
Wallet => WalletSigned or PrivateWallet

packages/providers/src/index.ts Outdated Show resolved Hide resolved
@luizstacio
Copy link
Member Author

I think it looks good, but want to discuss names more.

I feel like the primary name should be Wallet and then we'd name the signed version with private key using a different name.

Like: WalletPublic => Wallet Wallet => WalletSigned or PrivateWallet

One suggestion from @digorithm is to follow the terminology rust has that is WalletUnlocked and WalletLocked https://github.com/FuelLabs/fuels-rs/blob/14ac1b9feaa09fe64513aefec9115166b971148e/docs/src/wallets/access.md

@luizstacio luizstacio marked this pull request as ready for review October 30, 2022 18:14
QuinnLee
QuinnLee previously approved these changes Oct 30, 2022
Copy link
Member

@arboleya arboleya left a comment

Choose a reason for hiding this comment

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

Good stuff!

LGTM.

@luizstacio luizstacio merged commit 0e91213 into master Oct 31, 2022
@luizstacio luizstacio deleted the ls/create-public-wallet branch October 31, 2022 16:02
Copy link
Contributor

@camsjams camsjams left a comment

Choose a reason for hiding this comment

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

:shipit:

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.

4 participants