-
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: enhance the communication between SDK and wallet #3735
base: master
Are you sure you want to change the base?
feat: enhance the communication between SDK and wallet #3735
Conversation
* feat: implement flags for the transaction request * chore: combine flags for `isEstimated` and `isFunded` * chore: remove `isSigned` from witness updates * changeset * revert experimental changes * Aligned to the new re-spec * update to state * Update state * chore: fix connector test * lintfix * Apply suggestions from code review
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
* chore: moved chainInfo serialization * chore: added serialize chain info * move chain info helper * add deprecation notice * chore: added deserialize of nodeInfo * Added the ability to restore the provider chain + node info cache * consolidate serialization * serialize provider cache * pass serialized chain to connector * lintfix * lintfix * changeset * changeset * lintfix
packages/account/src/account.ts
Outdated
if (params.skipCustomFee) { | ||
const chainId = await this.provider.getChainId(); | ||
request.updateState(chainId, 'signed'); | ||
return { request, state: 'signed' }; | ||
} |
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.
Could there be cases where the skipCustomFee
flag is used, but the transaction is not yet signed?
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.
Yes, that could be a possibility.
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.
On top of the above, I also think a method named validate
should not mutate its received parameters.
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.
On top of the above, I also think a method named validate should not mutate its received parameters.
packages/account/src/providers/transaction-request/transaction-request.ts
Show resolved
Hide resolved
export type FuelConnectorSendTxParams = { | ||
skipCustomFee?: boolean; | ||
onBeforeSend?: (txRequest: TransactionRequest) => Promise<TransactionRequest>; | ||
provider?: { | ||
url: string; | ||
cache?: SerializedProviderCache; | ||
}; | ||
data?: { | ||
transactionSummary?: SerializedTransactionSummary; | ||
estimatedGasPrice?: string; | ||
latestGasPrice?: string; | ||
}; | ||
state?: TransactionStateFlag['state']; | ||
}; |
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.
I've noticed that the interface name being used here differs from the one being specified within this PR's description. Is this intentional?
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.
Do you mean the SerializedProviderCache
?
I wanted to show the full payload in the description so expanded the SerializedProviderCache
to get:
provider: {
url: string,
cache?: {
consensusParameterTimestamp?: string,
chain: SerializedChainInfo,
nodeInfo: SerializedNodeInfo,
}
},
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, just commented some nits.
Coverage Report:
Changed Files:
|
Release notes
In this release, we:
Summary
SendTransactionJsonRpc
specification:undefined
funded
signed
skipCustomFee
.Provider
using the serialised cache:Checklist