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.

Sign up for GitHub

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

Jump to bottom

Inverted Submission Flow #3665

Open
danielbate opened this issue Feb 5, 2025 · 6 comments · Fixed by #3689
Open

Inverted Submission Flow #3665

danielbate opened this issue Feb 5, 2025 · 6 comments · Fixed by #3689
Assignees
@petertonysmith94
Labels
feat Issue is a feature

Comments

@danielbate
Copy link
Contributor

Problem

When a transaction is submitted from a connector and approved in a user wallet, it takes 3 requests to submit a tx and build it's summary inside the application. Versus it only taking 1 request when submitting a tx from the SDK when we have the private key readily accessible.

The main reason for this is that as the wallet submits the tx, the application loses context of both the submission and finalised transaction, and therefore it must fetch both the raw tx and tx status to build it's summary.

This issue aims to bring parity to these 2 flows and enable single request submission and summary inside apps using connectors, as visualised below:

Image

Solution

To enable the above flow, the app must take ownership of tx submission. With the wallet passing back the finalised tx, propagated via the connectors.

This would involve the following work:

SDK

  1. Introduce a transaction preparation function to be utilised by connectors, via the connector interface. The expected signature is as follows:
  /**
   * Should perform all necessary operations (i.e estimation,
   * funding, signing) to prepare a tx so it can be submitted
   * at the app level.
   *
   * @param address - The address to sign the tx
   * @param transaction - The tx to prepare
   *
   * @returns The prepared tx request
   */
  async prepareForSend(address: string, transaction: TransactionRequestLike): Promise<TransactionRequest>;
  1. Utilise the preparation method inside Account.sendTransaction so that the process is still obfuscated to consumers.

fuels-ts/packages/account/src/account.ts

Lines 653 to 659 in 01375ce

async sendTransaction(
transactionRequestLike: TransactionRequestLike,
{ estimateTxDependencies = true, onBeforeSend, skipCustomFee = false }: AccountSendTxParams = {}
): Promise<TransactionResponse> {
if (this._connector) {
return this.provider.getTransactionResponse(
await this._connector.sendTransaction(this.address.toString(), transactionRequestLike, {

Non Native Wallets

For EVM and SVM wallets, all logic is inside the connector, so we need to do only the following

  1. Introduce the new preparation method inside the relevant connector, identical to sendTransaction however passing back the prepared tx rather than submitting

Native Wallets

Wallets such as Fuel Wallet and Bako, more of the logic is on the wallet side so would involve the following:

  1. Introduce the preparation method inside the wallet itself, with a new event to trigger the flow
  2. Introduce the method inside the connector, that submits the event
@danielbate danielbate self-assigned this Feb 5, 2025
@danielbate danielbate added the feat Issue is a feature label Feb 5, 2025
This was referenced Feb 6, 2025
Closed
Merged
@danielbate
Copy link
Contributor Author

Implementation is now complete as part of #3689 and was proved via the EVM and SVM connectors as can be seen here.

@danielbate danielbate mentioned this issue Feb 24, 2025
Open
@danielbate
Copy link
Contributor Author

In #3689 I introduced a usePrepareForSend flag to signal to Account.sendTransaction that this method should be used in the tx flow. FE did not like this approach and instead we should check for the presence of the function. I have been having trouble with making it optional on the abstract class, but let's see.

I'm going to revert #3869 so it is not included in 0.100.0 and we can agree the finalised approach with FE. As this will likely be similar to #3736, I agree with these going together anyway.

@danielbate danielbate reopened this Mar 6, 2025
This was referenced Mar 6, 2025
Merged
Closed
@danielbate
Copy link
Contributor Author

Blocked until #3669 and #3737 are complete.

@danielbate danielbate mentioned this issue Mar 18, 2025
Draft
4 tasks
@danielbate
Copy link
Contributor Author

danielbate commented Mar 20, 2025 •
edited
Loading

In #3669 we changed the FuelConnector.sendTransaction signature to return both a string | TransactionResponse:

fuels-ts/packages/account/src/connectors/fuel-connector.ts

Lines 47 to 51 in e8494fa

sendTransaction(
address: string,
transaction: TransactionRequestLike,
params?: FuelConnectorSendTxParams
): Promise<string | TransactionResponse>;

Although this does not encourage connectors or wallets to return a prepared request to give full flexibility to the SDK, it does enable the single request submit -> summary flow.

This may be enough for now and we can implement in the EVM/SVM connectors along with #3769.

But as preferred by @LuizAsFight, the Wallet will likely remain with the same 3 request flow for now.

I will prepare a draft PR to follow #3769 that supports this flow in docs/tests.

@arboleya
Copy link
Member

@danielbate Two quick questions:

  1. What are the three requests for the Wallet?
  2. Would this make the Wallet feel slower after users confirm? IIRC today it doesn't wait for anything and just closes very fast. Ideally, we didn't want to lose this snapiness.

@danielbate
Copy link
Contributor Author

danielbate commented Mar 20, 2025 •
edited
Loading

What are the three requests for the Wallet?

3 request flow from submission -> summary generation.

  • Wallet performs submitAndAwaitStatus
  • SDK performs statusChange
  • SDK performs getTransactionWithReceipts

Would this make the Wallet feel slower after users confirm? IIRC today it doesn't wait for anything and just closes very fast. Ideally, we didn't want to lose this snapiness.

No the wallet will remain as it is and will not lose it's snappiness. Only the App/SDK can be made snappier from this change.

@danielbate danielbate mentioned this issue Mar 20, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@petertonysmith94 petertonysmith94

Labels
feat Issue is a feature
Projects
None yet
Milestone
No milestone
3 participants
@arboleya @danielbate @petertonysmith94