-
Notifications
You must be signed in to change notification settings - Fork 11
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(ownership-provider): support payFee
and signTransaction
in sendTransaction
#272
Conversation
Codecov Report
@@ Coverage Diff @@
## main #272 +/- ##
==========================================
+ Coverage 74.80% 75.88% +1.08%
==========================================
Files 61 62 +1
Lines 1401 1497 +96
Branches 197 216 +19
==========================================
+ Hits 1048 1136 +88
- Misses 329 337 +8
Partials 24 24
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
we should mark all these functions as @internal
|
||
await provider.sendTransaction(txSkeleton); | ||
|
||
expect(provider.payFee).not.toHaveBeenCalled(); |
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.
This test made me realize that we should ban transactions with fees above a threshold, as this is usually the case with errors. We can do it in another issue
packages/ownership-providers/__tests__/FullOwnershipProvider.test.ts
Outdated
Show resolved
Hide resolved
packages/ownership-providers/__tests__/FullOwnershipProvider.test.ts
Outdated
Show resolved
Hide resolved
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
🚀 PR was released in |
Release Note
The
sendTransaction
is smarter now, we don't need to callprovider.payFee
andprovider.signTransaction
manually, just send itWhat Changed
Resolve #219
This PR make it is easier to use
FullOwnershipProvider#sendTransaction
, now when callsendTransaction
, payFee and sign will also be done if needed.Motivation
Please see #219 for more details.
Change Type
Indicate the type of change your pull request is:
documentation
patch
minor
major
🐤 Download canary assets:
nexus--canary.272.4869012901.zip
📦 Published PR as canary version:
0.0.19--canary.272.4869012901.0
✨ Test out this PR locally via: