-
Notifications
You must be signed in to change notification settings - Fork 45
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 prepareForSend
for EVM and SVM connector
#477
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
prepareForSend
for EVM and SVM connecotprepareForSend
for EVM and SVM connector
Coverage Report for Fuel Wallet (./packages/fuel-wallet)
File Coverage
|
Coverage Report for Fuel Development Wallet (./packages/fuel-development-wallet)
File CoverageNo changed files found. |
Coverage Report for Fuelet Wallet (./packages/fuelet-wallet)
File CoverageNo changed files found. |
@@ -56,6 +57,7 @@ export class EVMWalletConnector extends PredicateConnector { | |||
private _currentAccount: string | null = null; | |||
private config: EVMWalletConnectorConfig = {}; | |||
private _ethereumEvents = 0; | |||
usePrepareForSend = true; |
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.
@petertonysmith94 @danielbate
makes sense to the need for this flag? the sdk could just verify if prepareForSend
method exists
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.
The prepareForSend
method is on the abstract FuelConnector
class, and therefore, this method will always exist. Open to suggestions if you have another solution for this.
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.
it can't be optional?
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.
As it's an abstract method on the abstract class, it can't be optional. We could introduce a new interface that the underlying connector would implement, however, this could lead to unexpected behaviours with the Fuel SDK.
interface PrepareForSendConnector {
prepareForSend(
address: string,
transaction: TransactionRequestLike
): Promise<TransactionRequestLike>;
}
IMO the flag is simple and explicit.
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.
it can't be an optional method in the abstract class?
Summary
usePrepareForSend = true
flag to connectors to indicate support for the inverted flowChecklist