-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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: move predicate to work as a wallet #789
Conversation
Coverage report
Show new covered files 🐣
Test suite run success713 tests passing in 98 suites. Report generated by 🧪jest coverage report action from c6d7c9c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good on first pass, need to also add to typedoc generation and adjust address usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, some small questions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! 🚀
I've just left a single comment about type overloading (again). All other things I'd request have been requested from others reviewers and implemented already.
The only noteworthy exception is that we don't need to add a new package for this code split. We can DRY stuff by keeping the account.ts
and account.test.ts
inside the wallet
package, removing many boilerplate files that don't need to exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@luizstacio Can you please move the account.ts
and account.test.ts
files inside the wallet
package instead of creating a new package for holding only these two files?
Let's try to DRY things, reducing the amount of boilerplate code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful. ❤️
I left some final and minor suggestions regarding the changelogs. ✨
Co-authored-by: Anderson Arboleya <anderson@arboleya.me>
Co-authored-by: Anderson Arboleya <anderson@arboleya.me>
Co-authored-by: Anderson Arboleya <anderson@arboleya.me>
This PR enables Predicate to be used as a Wallet, enabling predicates to be passed to contract instances, do transfers, and pay for transactions.
On this PR;
BaseWalletLocked
toAccount
.Predicate
and add the same API ofAccount
by extending it.close: #788