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

Bug: React Native sendTransaction something went wrong error #242

Open
bearsworth opened this issue Jul 31, 2023 · 17 comments
Open

Bug: React Native sendTransaction something went wrong error #242

bearsworth opened this issue Jul 31, 2023 · 17 comments
Labels
type: bug Something isn't working

Comments

@bearsworth
Copy link

bearsworth commented Jul 31, 2023

Describe the bug

When using sendTransaction for networks such as Polygon Mumbai, something went wrong error occurs

Steps

Use new react native project.
Follow the guide.
Everything works as expected such as request accounts, switch chains.
Proceed to use sendTransaction / signTransaction on Polygon Mumbai / Polygon network.

Get message in Coinbase Wallet app -> something went wrong... sign out etc.

Below is how the request is being made for eth_sendTransaction

const result = await provider.request({ method: 'eth_sendTransaction', params: [{ from: 'my polygon address', to: 'my polygon address', value: '50000', }], });

For value I have used hex value and integer as well with no changes.

What I do before running this request is successfully switch the polygon network on the app via request. I then proceed to do the above request.

Expected behavior

A message should appear to sign or send Transaction in the Coinbase app.

Version

1.0.7

Additional info

No response

Desktop

No response

Smartphone

Testing on physical device iPhone 16.5.1c.

@bearsworth bearsworth added the type: bug Something isn't working label Jul 31, 2023
@bearsworth
Copy link
Author

bearsworth commented Aug 2, 2023

I have attempted to try many different thing such as change values and ensuring they match what is required. I have also attempted to convert values to BN for wei values and gas amounts.

My only thoughts are that it's possible the coinbase wallet app might not allow signing/sending from specific ips like local host ones.

My project is a dev build from expo with the following up -> exp+myapp://192.168.0.166:8081

Deeplink urls work properly as I can sign personal, switch networks, add networks.

I have also tried adjusting the gas amounts because I have read the 'Something went wrong error' could be because you do not have enough funds for the fees. However, I have set fees and gas amounts much higher than required to no avail and I can send testnet funds to myself as a test on the app itself (so I have enough funds to send).

If anyone has info about sendTransaction for react native, would appreciate anyone just sending how you were able to achieve this.

@hamberluo
Copy link

same problem using flutter

@bangtoven
Copy link
Member

hey. sorry for the late response.
thanks for sharing the details.
seems there's some error with cbwallet's validation logic. well, it should have shown more descriptive error message than Something went wrong error. will investigate.

  • fyi, (it sounds like you were sending them after handshake, but just in case) ["eth_signTransaction", "eth_sendTransaction"] requests are not allowed during handshake initial requests.
  • i don't think there's any restriction around using local network.

@bearsworth
Copy link
Author

hey. sorry for the late response. thanks for sharing the details. seems there's some error with cbwallet's validation logic. well, it should have shown more descriptive error message than Something went wrong error. will investigate.

  • fyi, (it sounds like you were sending them after handshake, but just in case) ["eth_signTransaction", "eth_sendTransaction"] requests are not allowed during handshake initial requests.
  • i don't think there's any restriction around using local network.

Thanks for the update. I believe I'm getting this error post hand shake. We have the test cb wallet account switch networks which it does, then request send transaction which shows something is wrong.

@bearsworth
Copy link
Author

bearsworth commented Aug 14, 2023

hey. sorry for the late response. thanks for sharing the details. seems there's some error with cbwallet's validation logic. well, it should have shown more descriptive error message than Something went wrong error. will investigate.

  • fyi, (it sounds like you were sending them after handshake, but just in case) ["eth_signTransaction", "eth_sendTransaction"] requests are not allowed during handshake initial requests.
  • i don't think there's any restriction around using local network.

Could you test whether we can send polygon mumbai (testnet) matic without gettting the 'something went wrong' that would make sure its all good.

I haven't tested between now and since I posted this, but I can try again. All I know is that I have metamask working and
you have to do something similar and this part of the wallet for my app just fails regardless.

@bangtoven
Copy link
Member

it might be because some of the fields are missing.
can you make sure to pass these?

case eth_sendTransaction(
        fromAddress: EthAddress,
        toAddress: EthAddress?,
        weiValue: BigInt,
        data: EthTxData,
        nonce: Int?,
        gasPriceInWei: BigInt?,
        maxFeePerGas: BigInt?,
        maxPriorityFeePerGas: BigInt?,
        gasLimit: BigInt?,
        chainId: BigInt
    )

@bearsworth
Copy link
Author

bearsworth commented Aug 14, 2023

I'll give it a go but a few things.

First, I don't think a nonce is required. Second I think the chainId is set by the app.

In the mobile sdk it goes to your private func: _prepareTransactionParams:

You will notice that chainId is also not a parameter to be set by us but fetched from the sdk once we change networks or defaulted to eth.

  private _prepareTransactionParams(tx: {
    from?: unknown;
    to?: unknown;
    gasPrice?: unknown;
    maxFeePerGas?: unknown;
    maxPriorityFeePerGas?: unknown;
    gas?: unknown;
    value?: unknown;
    data?: unknown;
    nonce?: unknown;
  }): EthereumTransactionParams {
    const fromAddress = tx.from ? ensureAddressString(tx.from) : null;
    if (!fromAddress) {
      throw new Error("Ethereum address is unavailable");
    }

    const toAddress = tx.to ? ensureAddressString(tx.to) : null;
    const weiValue = tx.value != null ? ensureBN(tx.value) : new BN(0);
    const data = tx.data ? ensureBuffer(tx.data) : Buffer.alloc(0);
    const nonce = tx.nonce != null ? ensureIntNumber(tx.nonce) : null;
    const gasPriceInWei = tx.gasPrice != null ? ensureBN(tx.gasPrice) : null;
    const maxFeePerGas =
      tx.maxFeePerGas != null ? ensureBN(tx.maxFeePerGas) : null;
    const maxPriorityFeePerGas =
      tx.maxPriorityFeePerGas != null
        ? ensureBN(tx.maxPriorityFeePerGas)
        : null;
    const gasLimit = tx.gas != null ? ensureBN(tx.gas) : null;
    const chainId = this._chainId
      ? IntNumber(this._chainId)
      : this._getChainId();

    return {
      fromAddress,
      toAddress,
      weiValue,
      data,
      nonce,
      gasPriceInWei,
      maxFeePerGas,
      maxPriorityFeePerGas,
      gasLimit,
      chainId,
    };
  }

In about an hour I'll run some tests though again to see if it works.

@bearsworth
Copy link
Author

bearsworth commented Aug 14, 2023

To add to above -> In the case we don't provide the values, the sdk has default values set for us (you guys use sign for both send and sign)

private async _eth_signTransaction(
    params: unknown[],
    shouldSubmit: boolean
  ): Promise<JSONRPCResponse> {
    this._requireAuthorization();
    const tx = this._prepareTransactionParams((params[0] as any) || {});
    const action: Action = {
      method: shouldSubmit
        ? JSONRPCMethod.eth_sendTransaction
        : JSONRPCMethod.eth_signTransaction,
      params: {
        fromAddress: tx.fromAddress,
        toAddress: tx.toAddress,
        weiValue: bigIntStringFromBN(tx.weiValue),
        data: hexStringFromBuffer(tx.data),
        nonce: tx.nonce,
        gasPriceInWei: tx.gasPriceInWei
          ? bigIntStringFromBN(tx.gasPriceInWei)
          : null,
        maxFeePerGas: tx.maxFeePerGas
          ? bigIntStringFromBN(tx.maxFeePerGas)
          : null,
        maxPriorityFeePerGas: tx.maxPriorityFeePerGas
          ? bigIntStringFromBN(tx.maxPriorityFeePerGas)
          : null,
        gasLimit: tx.gasLimit ? bigIntStringFromBN(tx.gasLimit) : null,
        chainId: tx.chainId,
      },
    };

    const res = await this._makeSDKRequest(action);
    return {
      jsonrpc: "2.0",
      id: 0,
      result: res,
    };
  }

@bearsworth
Copy link
Author

bearsworth commented Aug 16, 2023

it might be because some of the fields are missing. can you make sure to pass these?

case eth_sendTransaction(
        fromAddress: EthAddress,
        toAddress: EthAddress?,
        weiValue: BigInt,
        data: EthTxData,
        nonce: Int?,
        gasPriceInWei: BigInt?,
        maxFeePerGas: BigInt?,
        maxPriorityFeePerGas: BigInt?,
        gasLimit: BigInt?,
        chainId: BigInt
    )

I have used values for this. I am still getting the error after testing.

I tried to send polygon mumbai matic to myself.

let val = '8000000000000000';
val = ethers.BigNumber.from(val);
val = ethers.utils.hexlify(val);

this is an example above of how I'm formatting the values. I have tried with straight BigNumber and converting with hexlify.
In metamask these values work.

Also for values such as gasPrice -> I used Alchemy SDK to get the current gasPrice for the polygon mumbai network.

I can't seem to get these to work with the Coinbase mobile app though. I have enough funds to send.

params: [{
from: coinbaseAddress,
to: coinbaseAddress,
value: val,
data: '0x',
nonce: 0,
gasPrice,
gas: gasLimit,
maxFeePerGas,
maxPriorityFeePerGas: maxFeePerGas,
}],

If you would not mind, maybe you can run a test and let me know what values you used but any test I've tried and played with for a few hours always get the something went wrong page.

@AmjadKhan2k18
Copy link

Hey everyone, I has same issue in Flutter and I fixed it, I am going to create a PR it will fix the issue, I think it will be same for RN too

@bearsworth
Copy link
Author

Hey everyone, I has same issue in Flutter and I fixed it, I am going to create a PR it will fix the issue, I think it will be same for RN too

Possible to share what you did to make it work? Does it alter the sdk itself?

@AmjadKhan2k18
Copy link

@bearsworth here is PR sorry I was lil busy and forgot to create PR

#251

changes I make to fix sendTransaction is

make toAddress required
nonce is expected when sending transaction but it shouldn't be, I don't use node in FE so I provided nonce 0 when sending transaction

@bearsworth
Copy link
Author

bearsworth commented Sep 12, 2023

@bearsworth here is PR sorry I was lil busy and forgot to create PR

#251

changes I make to fix sendTransaction is

make toAddress required nonce is expected when sending transaction but it shouldn't be, I don't use node in FE so I provided nonce 0 when sending transaction

Thanks for the response. Some of the changes you made I think I was still submitting the same data so I doubt it would fix anything.

Could you try sending test polygon mumbai to yourself to verify?

Also if possible could you share with me which values you used?

The changes you submitted I believe I have used the same type of data, and also used a nonce value but I get the same error for some odd reason.

I just realized this but also, the SDK for react native appears different.

You see it uses eth_signTransaction and also it automatically creates strings for us for the wei value:

yes ->sign and send transaction for the sdk is the same - they decided to make one function do both because it appears they share the same params.

private async _eth_signTransaction(
params: unknown[],
shouldSubmit: boolean
): Promise {
this._requireAuthorization();
const tx = this._prepareTransactionParams((params[0] as any) || {});
const action: Action = {
method: shouldSubmit
? JSONRPCMethod.eth_sendTransaction
: JSONRPCMethod.eth_signTransaction,
params: {
fromAddress: tx.fromAddress,
toAddress: tx.toAddress,
weiValue: bigIntStringFromBN(tx.weiValue),
data: hexStringFromBuffer(tx.data),
nonce: tx.nonce,
gasPriceInWei: tx.gasPriceInWei
? bigIntStringFromBN(tx.gasPriceInWei)
: null,
maxFeePerGas: tx.maxFeePerGas
? bigIntStringFromBN(tx.maxFeePerGas)
: null,
maxPriorityFeePerGas: tx.maxPriorityFeePerGas
? bigIntStringFromBN(tx.maxPriorityFeePerGas)
: null,
gasLimit: tx.gasLimit ? bigIntStringFromBN(tx.gasLimit) : null,
chainId: tx.chainId,
},
};

@AmjadKhan2k18
Copy link

@bearsworth I have been testing with ETH, POL, AVAX, BNB chain and it's working fine, I haven't tested with test chains, I will share trx Params I have sent in transaction right now I am outside

@bearsworth
Copy link
Author

@bearsworth I have been testing with ETH, POL, AVAX, BNB chain and it's working fine, I haven't tested with test chains, I will share trx Params I have sent in transaction right now I am outside

Thank you. I think one of the differences I noticed between the flutter and react-native one is that you set it to a generic string vs the react-native one sets it to a BN string. Maybe that might be an issue, but later I will try and change the sdk to see for the react-native case.

@bearsworth
Copy link
Author

bearsworth commented Sep 20, 2023

@bangtoven

The issue is apparently a bug with WalletMobileSDKEVMProvider -> more specifically how the SDK sets the chain Id.
The issue is in the function below.

private _prepareTransactionParams(tx: {
    from?: unknown;
    to?: unknown;
    gasPrice?: unknown;
    maxFeePerGas?: unknown;
    maxPriorityFeePerGas?: unknown;
    gas?: unknown;
    value?: unknown;
    data?: unknown;
    nonce?: unknown;
  }): EthereumTransactionParams {
    const fromAddress = tx.from ? ensureAddressString(tx.from) : null;
    if (!fromAddress) {
      throw new Error("Ethereum address is unavailable");
    }
    const toAddress = tx.to ? ensureAddressString(tx.to) : null;
    const weiValue = tx.value != null ? ensureBN(tx.value) : new BN(0);
    const data = tx.data ? ensureBuffer(tx.data) : Buffer.alloc(0);
    const nonce = tx.nonce != null ? ensureIntNumber(tx.nonce) : null;
    const gasPriceInWei = tx.gasPrice != null ? ensureBN(tx.gasPrice) : null;
    const maxFeePerGas =
      tx.maxFeePerGas != null ? ensureBN(tx.maxFeePerGas) : null;
    const maxPriorityFeePerGas =
      tx.maxPriorityFeePerGas != null
        ? ensureBN(tx.maxPriorityFeePerGas)
        : null;
    const gasLimit = tx.gas != null ? ensureBN(tx.gas) : null;
    const chainId = this._chainId
      ? IntNumber(this._chainId)
      : this._getChainId();
    return {
      fromAddress,
      toAddress,
      weiValue,
      data,
      nonce,
      gasPriceInWei,
      maxFeePerGas,
      maxPriorityFeePerGas,
      gasLimit,
      chainId,
    };
  }
const chainId = this._chainId
      ? IntNumber(this._chainId)
      : this._getChainId();

This returns an integer when the API for the Coinbase Wallet app is accepting strings. So it will always throw an error.

I was originally going to try and submit a fix, but it looks like the type set by your team is an integer so it looks like there has to be some more thought about the resolution.

For anyone that needs a simple patch package fix do this to the chainId:

const chainId = prepend0x(this._getChainId().toString(16));

My other thought was you guys should let users enter their own chainId for checkout as another param.

For now I have patched the package, but If anyone runs across this, you need to change this to submit send requests and also sign Transactions.

@bearsworth
Copy link
Author

@bearsworth I have been testing with ETH, POL, AVAX, BNB chain and it's working fine, I haven't tested with test chains, I will share trx Params I have sent in transaction right now I am outside

Turns out it was a bug with the React native library side, it looks like we have to change different stuff for each one, but without anyone submitting stuff, I would not have attempted to even try to figure it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants
@bangtoven @hamberluo @bearsworth @AmjadKhan2k18 and others