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.

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

Submit from wallet with fee payer support #189

Closed
wants to merge 1 commit into from

Conversation

gregnazario
Copy link
Contributor

@gregnazario gregnazario commented Aug 31, 2023

Followup from previous fee payer PR, which adds support for submitting from a wallet directly.

I'm still working on testing it with Petra or another wallet directly to ensure it is the right model.

aptos-labs/aptos-core#9896

This has been tested with petra as a wallet plugin here aptos-labs/petra-plugin-wallet-adapter#10

Copy link
Collaborator

@0xmaayan 0xmaayan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you run changeset so we include it in next version release

@@ -501,4 +487,113 @@ export class WalletCore extends EventEmitter<WalletCoreEvents> {
throw new WalletSignMessageAndVerifyError(errMsg).message;
}
}

async prepRawTransaction(input: RawTransactionPrepPayload): Promise<AnyRawTransaction> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add some comments on what this function does

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof, I forgot documentation in my mad rush, let me fix that

}
}

async signRawTransaction(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this can be used for any transaction single/feepayer/agent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

}
}

private getClient(): Provider {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the need for this function?

Copy link
Contributor Author

@gregnazario gregnazario Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used above.

Basically, the sequence number and chain ID need to be pulled prior to generating the transaction. This can alternatively be sent to the wallet instead of doing this.

@gregnazario gregnazario force-pushed the fee-payer-submit branch 2 times, most recently from b63f928 to 3575f8e Compare October 30, 2023 22:34
Copy link
Collaborator

@0xmaayan 0xmaayan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document those changes in https://github.com/aptos-labs/aptos-wallet-adapter/tree/main/packages/wallet-adapter-react

tbh, this becomes very complex, and with the new v2 support it would become even more complex. I think we need to start working on adapter v2 and then push wallets to support v2. We are going to end up with 3 different functions to submit a transaction

@gregnazario
Copy link
Contributor Author

We should document those changes in https://github.com/aptos-labs/aptos-wallet-adapter/tree/main/packages/wallet-adapter-react

tbh, this becomes very complex, and with the new v2 support it would become even more complex. I think we need to start working on adapter v2 and then push wallets to support v2. We are going to end up with 3 different functions to submit a transaction

Unfortunately there is always going to be minimally a 3 step process to submitting a transaction.

  1. Generate the transaction (we can actually do this locally), but I've added it here for convenience so that users don't have to reimplement it. The new SDK can just handle this on it's own.
  2. Sign by one party (the primary signer, or the fee payer)
  3. Sign and submit by the other party (primary signer or the fee payer)

This could be reduced down to mostly client logic, but this is to simplify the process since the fee payer key will have to be out of the client anyways. If a wallet decides to sponsor the transactions itself, it can possibly skip over some of that to one call by the user.

@0xmaayan
Copy link
Collaborator

We should document those changes in https://github.com/aptos-labs/aptos-wallet-adapter/tree/main/packages/wallet-adapter-react
tbh, this becomes very complex, and with the new v2 support it would become even more complex. I think we need to start working on adapter v2 and then push wallets to support v2. We are going to end up with 3 different functions to submit a transaction

Unfortunately there is always going to be minimally a 3 step process to submitting a transaction.

  1. Generate the transaction (we can actually do this locally), but I've added it here for convenience so that users don't have to reimplement it. The new SDK can just handle this on it's own.
  2. Sign by one party (the primary signer, or the fee payer)
  3. Sign and submit by the other party (primary signer or the fee payer)

This could be reduced down to mostly client logic, but this is to simplify the process since the fee payer key will have to be out of the client anyways. If a wallet decides to sponsor the transactions itself, it can possibly skip over some of that to one call by the user.

I was mostly talking about signAndSubmitTransaction, signAndSubmitRawTransaction, submitTransaction (v2)

This adds 3 new APIs, prepRawTransaction, signRawTransaction, and
signAndSubmitRawTransaction.  Prep will create a multiagent or fee
payer transaction accordingly, unsigned with information from the
wallet on sequence number and chain id.  Then all parties can sign
the transaction with signRawTransaction, and the last party can
sign with signAndSubmitRawTransaction to sign and send it off as the
sender.
@gregnazario
Copy link
Contributor Author

We should document those changes in https://github.com/aptos-labs/aptos-wallet-adapter/tree/main/packages/wallet-adapter-react
tbh, this becomes very complex, and with the new v2 support it would become even more complex. I think we need to start working on adapter v2 and then push wallets to support v2. We are going to end up with 3 different functions to submit a transaction

Unfortunately there is always going to be minimally a 3 step process to submitting a transaction.

  1. Generate the transaction (we can actually do this locally), but I've added it here for convenience so that users don't have to reimplement it. The new SDK can just handle this on it's own.
  2. Sign by one party (the primary signer, or the fee payer)
  3. Sign and submit by the other party (primary signer or the fee payer)

This could be reduced down to mostly client logic, but this is to simplify the process since the fee payer key will have to be out of the client anyways. If a wallet decides to sponsor the transactions itself, it can possibly skip over some of that to one call by the user.

I was mostly talking about signAndSubmitTransaction, signAndSubmitRawTransaction, submitTransaction (v2)

Yes, we need to collapse them into 1 for the new SDK and the future of the API.

We could just drop this and merge it in with the rest on the outside interface, but the wallets still need the inner interface.

@gregnazario
Copy link
Contributor Author

Closing in favor of SDK v2 bindings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants