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

Wallet adapter fails to use fullnode url at submitTransaction #392

Open
JackyWYX opened this issue Aug 6, 2024 · 3 comments
Open

Wallet adapter fails to use fullnode url at submitTransaction #392

JackyWYX opened this issue Aug 6, 2024 · 3 comments

Comments

@JackyWYX
Copy link

JackyWYX commented Aug 6, 2024

Description

Code

During submitTransaction, a new AptosConfig is created, but the new config does not include the optional fullnode field in the config data structure, which will lead to fullnode config does not take effect when submitting transaction.

  async submitTransaction(
    transaction: InputSubmitTransactionData
  ): Promise<PendingTransactionResponse> {
    ...
      const aptosConfig = new AptosConfig({
        network: convertNetwork(this.network),
      });
      ...

Proposed solution

Add optional fullnode url and other optional settings from AptosConfig when creating the new AptosConfig at submitTransaction

@0xmaayan
Copy link
Collaborator

0xmaayan commented Aug 6, 2024

Hi @JackyWYX , thanks for reporting.

The adapter logic is - first check the wallet supports submitTransaction, if it does it would use the network set on the wallet.

But when a wallet doesnt support that feature, then the adapter assumes only Aptos networks. I think adding optional fullnode (and some other configs like indexerurl, etc) to the adapter APIs makes sense - we would need it on the Provider level and on AdapterCore for some APIs - submitTransaction, signTransaction , signAndSubmitTransaction

@zhshy11
Copy link

zhshy11 commented Aug 7, 2024

Hi @JackyWYX , thanks for reporting.

The adapter logic is - first check the wallet supports submitTransaction, if it does it would use the network set on the wallet.

But when a wallet doesnt support that feature, then the adapter assumes only Aptos networks. I think adding optional fullnode (and some other configs like indexerurl, etc) to the adapter APIs makes sense - we would need it on the Provider level and on AdapterCore for some APIs - submitTransaction, signTransaction , signAndSubmitTransaction

Hello, do you have a time schedule?

@0xmaayan
Copy link
Collaborator

0xmaayan commented Aug 7, 2024

Hi @JackyWYX , thanks for reporting.
The adapter logic is - first check the wallet supports submitTransaction, if it does it would use the network set on the wallet.
But when a wallet doesnt support that feature, then the adapter assumes only Aptos networks. I think adding optional fullnode (and some other configs like indexerurl, etc) to the adapter APIs makes sense - we would need it on the Provider level and on AdapterCore for some APIs - submitTransaction, signTransaction , signAndSubmitTransaction

Hello, do you have a time schedule?

could probably only start working on it mid-late September - are you open to submit a PR for that?

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

No branches or pull requests

3 participants