airgap-it / beacon-sdk

The beacon sdk allows developers of dApps and wallets on Tezos to implement the wallet interaction standard tzip-10.
https://walletbeacon.io
MIT License
103 stars 65 forks source link

Conversion issue when amount exceed max int #790

Closed alexperronnet closed 3 months ago

alexperronnet commented 4 months ago

Describe the bug (current behavior) When the amount to be sent exceeds the max integer value (2147483647), the conversion does not display the correct amount. For example, sending 200 Tez (200,000,000 mutez) works correctly, but sending 400 Tez (400,000,000 mutez) results in an incorrect conversion.

To Reproduce Steps to reproduce the behavior:

  1. Go to the function sendTezosToStables.
  2. Set the amount to be sent to a value exceeding 2147483647 in mutez.
  3. Attempt to send the amount.
  4. See error in the conversion amount.

Expected (correct) behavior The amount should be converted correctly regardless of its size, ensuring accurate transactions.

Additional context

export async function sendTezosToStables(props: SendTezosToStablesInput) {
  const activeAccount = await dAppClient.getActiveAccount()
  if (!activeAccount) {
    await dAppClient.requestPermissions()
    return sendTezosToStables(props)
  }
  const TEZOS_IN_MUTEZ = 1_000_000

  const operation: PartialTezosOperation = {
    kind: TezosOperationType.TRANSACTION,
    destination: PUBLIC_WALLET_ADDRESS,
    amount: "403000000",
    source: props.source
  }
  return dAppClient.requestOperation({ operationDetails: [operation] })
}

Preview

davaxi commented 4 months ago

+1

IsaccoSordo commented 4 months ago

Hi @alexperronnet , Thank you for raising the issue. While sending 200 tez and 400 tez works fine, it is true that any amount exceeding Number.MAX_SAFE_INTEGER would be wrongly converted. However, since such a number would be greater than 9 quadrillion, a transaction this large would likely fail. Therefore, we have decided to add a condition to check the amount before sending the operation.

AndreasGassmann commented 3 months ago

Hi @alexperronnet and @davaxi. As @IsaccoSordo mentioned, numbers exceeding 2147483647 should work up to 9007199254740991, which is the MAX_SAFE_INTEGER value.

The beacon-sdk actually does not modify or validate any data that is sent. That is either taquito on the client side (if you use taquito), or the wallet itself. Beacon is only the transport layer.

In your example you say that 200 tez worked, but 400 tez didn't. But the max int 2147483647 would actually be 2147 tez. Also, in your example you are trying to send 403'000'000 tez (which would be 403'000'000'000'000 mutez), which is still smaller than MAX_SAFE_INTEGER and therefore would still work (assuming anyone had that much tez).

Seeing this, it's highly unlikely that you ran into a conversion issue on the beacon side of things. Please feel free to provide additional information so we can reproduce this issue, otherwise we will close it in a few days.