hashgraph / hedera-sdk-js

Hedera™ Hashgraph SDK for JavaScript/TypeScript
https://docs.hedera.com/guides/docs/sdks
Apache License 2.0
272 stars 144 forks source link

bug: TokenAssociateTransaction#setAccountId does not accept EVM address based account ID #2442

Open bguiz opened 3 months ago

bguiz commented 3 months ago

Description

Variation 1 - EVM address as string:

const assocTx = await new TokenAssociateTransaction()
        .setAccountId('0x34fe5d75bd7ee95273ac9883c460d71edb0a2b6e')
        .setTokenIds([tokenId])
        .setNodeAccountIds([AccountId.fromString('0.0.5')])
        .freezeWith(client);

Variation 2 - account ID based on EVM address:

const assocTx = await new TokenAssociateTransaction()
        .setAccountId(AccountId.fromEvmAddress(0, 0,'0x34fe5d75bd7ee95273ac9883c460d71edb0a2b6e'))
        .setTokenIds([tokenId])
        .setNodeAccountIds([AccountId.fromString('0.0.5')])
        .freezeWith(client);

Variation 3 - Account ID in S.R.N format:

const assocTx = await new TokenAssociateTransaction()
        .setAccountId(AccountId.fromString('0.0.4654396'))
        .setTokenIds([tokenId])
        .setNodeAccountIds([AccountId.fromString('0.0.5')])
        .freezeWith(client);

Steps to reproduce

  1. Create assocTx in each of the above variations.
  2. Sign an submit assocTx using the code below
  3. Observe that with variation 1 and 2, there is an error, but variation 3 works
    const assocTxId = assocTx.transactionId;
    logger.log('The token association transaction ID:', assocTxId.toString());

    await logger.logSectionWithWaitPrompt('Submitting token association transaction');
    const assocTxSigned = await assocTx.sign(account1Key);

    const assocTxSubmitted = await assocTxSigned.execute(client);

    const assocTxReceipt = await assocTxSubmitted.getReceipt(client);
    const assocTxStatus = assocTxReceipt.status;
    logger.log(
        'The token association transaction status is:',
        assocTxStatus.toString(),
    );

Additional context

The error is INVALID_ACCOUNT_ID and the full output is copied below. The error occurs when getReceipt is invoked.

 ReceiptStatusError: receipt for transaction 0.0.4650263@1722847623.102513022 contained error status INVALID_ACCOUNT_ID
    at new ReceiptStatusError (file:///Users/user/code/hedera/hedera-tokens-cyoa-tutorial/node_modules/@hashgraph/sdk/src/ReceiptStatusError.js:37:9)
    at TransactionReceiptQuery._mapStatusError (file:///Users/user/code/hedera/hedera-tokens-cyoa-tutorial/node_modules/@hashgraph/sdk/src/transaction/TransactionReceiptQuery.js:332:16)
    at TransactionReceiptQuery.execute (file:///Users/user/code/hedera/hedera-tokens-cyoa-tutorial/node_modules/@hashgraph/sdk/src/Executable.js:725:32)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async TransactionResponse.getReceipt (file:///Users/user/code/hedera/hedera-tokens-cyoa-tutorial/node_modules/@hashgraph/sdk/src/transaction/TransactionResponse.js:81:25)
    at async scriptTokenHts (file:///Users/user/code/hedera/hedera-tokens-cyoa-tutorial/tokenHts/script-tokenHts.js:126:28) {
  status: Status { _code: 15 },
  transactionId: TransactionId {
    accountId: AccountId {
      shard: [Long],
      realm: [Long],
      num: [Long],
      aliasKey: null,
      evmAddress: null,
      _checksum: null
    },
    validStart: Timestamp { seconds: [Long], nanos: [Long] },
    scheduled: false,
    nonce: null
  },
  transactionReceipt: TransactionReceipt {
    status: Status { _code: 15 },
    accountId: null,
    fileId: null,
    contractId: null,
    topicId: null,
    tokenId: null,
    scheduleId: null,
    exchangeRate: ExchangeRate {
      hbars: 30000,
      cents: 143313,
      expirationTime: 2024-08-05T09:00:00.000Z,
      exchangeRateInCents: 4.7771
    },
    topicSequenceNumber: Long { low: 0, high: 0, unsigned: false },
    topicRunningHash: Uint8Array(0) [],
    totalSupply: Long { low: 0, high: 0, unsigned: false },
    scheduledTransactionId: null,
    serials: [],
    duplicates: [],
    children: []
  }
}

Hedera network

testnet

Version

2.48.1

Operating system

macOS

a-ridley commented 2 months ago

This bug is also occurring on TokenFreezeTransaction() It was reported by a dev who is part of the Hello Future Hackathon.

TokenFreezeTransactionBug

Error Output:

tokenfreeze-bug-output
bguiz commented 2 months ago

Thanks @a-ridley - I think this indicates that a broader review is necessary, for all the SDK methods which accept an account ID as an input parameter, and identify which ones accept an EVM address it the place of the account ID, and which ones don't.

For those, there could be 2 possible fixes:

(1) Faster solution: Implement a util function to convert the EVM address to account ID conversion. Either locally using a mathematical conversion using "long-zero" EVM address, or by making a network request (e.g. to a mirror node API) to obtain the "standard" EVM address. ... and then use that as a substitute prior to invoking the HAPI on the Hedera nodes.

(2) Slower solution: Implement the lookup/ conversion in the HAPI implementation on the Hedera nodes.

This bug is also occurring on TokenFreezeTransaction() It was reported by a dev who is part of the Hello Future Hackathon.

ivaylonikolov7 commented 2 months ago

Hello, @bguiz @a-ridley

@bguiz, I agree with your given solutions but they have their complications.

About your first solution. There is no way to derive an account ID from the EVM Address. They are not generated using mathematical formula so the only way you can get the account ID is using the mirror node. As you mentioned this might make the transactions slow and we already did something similar. The community wasn't happy with the changes so we had to revert them. Also there would be some inconsistencies in how the SDK handles transactions which I am not really a fan of.

Your second solution seems better to me but it had some issues too. I found that it used to work like that until this PR . This PR addressed this issue. According to this issue: Using aliases in functions other than crypto transfer transaction was deprecated because we were not supporting aliases everywhere in mono-service.

As of right now I think there is no better way than using mirror node API. (edited)

bguiz commented 2 months ago

💯 agreed, I think that this is the best solution among the various options. So the question is about whether the SDK will automatically call the relevant mirror node API for you. If this is feasible, I think we should do it.

As of right now I think there is no better way than using mirror node API.