hashgraph / hedera-sdk-js

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

TokenCreateTransaction().setAutoRenewAccountId(accountId) throws INVALID_SIGNATURE #2291

Closed quiet-node closed 1 month ago

quiet-node commented 1 month ago

Description

There's a bump PR in the hedera-smart-contracts repo that bumps the SDK from 2.41.0 to 2.45.0 and this CI test is failing. Digging down I found out that it fails because of this .setAutoRenewAccountId(autoRenewAccount) method at this line. Currently, the autoRenewAccount is one of the wallets pre-configured in the Hardhat project.

After couple of tries, I figured that the .setAutoRenewAccountId() will not work with random accountId, but ONLY works with the operatorAccountId which can be retrieved by client.operatorAccountId(). This behavior didn't just happen in 2.45.0 but also in 2.42.0. I was looking at the changelog from 2.41.0 to 2.42.0 but didn't find anything that can possibly caused the problem.

Steps to reproduce

  1. You can either run the acceptance test in the smart-contract , or run the script below
    
    const {
    TokenCreateTransaction,
    TransactionId,
    PublicKey,
    TokenSupplyType,
    AccountId,
    Client,
    } = require('@hashgraph/sdk');

const main = async () => { const client = Client.forTestnet().setOperator( OPERATOR_ID, OPERATOR_KEY ); const notOperatorId = 0.0.2662570

const tokenCreate = await new TokenCreateTransaction() .setTokenName(tokenName) .setTokenMemo(tokenMemo) .setTokenSymbol(tokenSymbol) .setDecimals(3) .setInitialSupply(6) .setMaxSupply(9) .setSupplyType(TokenSupplyType.Finite) .setTreasuryAccountId(client.operatorAccountId) .setAutoRenewAccountId(notOperatorId) .setFreezeDefault(false) .setTransactionId(TransactionId.generate(client.operatorAccountId)) .setNodeAccountIds([client._network.getNodeAccountIdsForExecute()[0]]) .setTransactionMemo('Token') .execute(client);

const receipt = await tokenCreate.getReceipt(client); const tokenId = receipt.tokenId.toString(); console.log(tokenId); };

main();



2. Observe the `INVALID_SIGNATURE` error
3. Replace the `notOperatorId` value in .setAutoRenewAccountId()` function with `client.operatorAccountId` -> run again
4. Observe the `tokenId` is logged in the console

### Additional context

_No response_

### Hedera network

mainnet, testnet, previewnet, other

### Version

^v2.42.0

### Operating system

None
svetoslav-nikol0v commented 1 month ago

Hi @quiet-node, When the AutoRenewAccountId is set the transaction must be signed with the key of the AutoRenewAccountId. In your case the transaction is signed with the opratorKey by default that's why you're getting INVALID_SIGNATURE error.

quiet-node commented 1 month ago

Hi @svetoslav-nikol0v, you're right and I noticed it, and yes it makes perfect sense. What's strange is that before v2.42.0, the .setAutoRenewAccountId() worked with the notOperatorId. Are we saying that it was a bug, and now it got fixed in v2.42.0? If so, could you please confirm and if possible could you point out where the fix happened? If not prob verbal confirmation is good enough. I need a refference for the fix PR in the hedera-smart-contract repos.

SimiHunjan commented 1 month ago

The difference could be that on testnet we have modularized code enabled which is 0.49. This is the version that enables the refactoring work that took place over the last year and more. It could be the case that the code previous to 0.49 did not require the auto renew account to sign if you applied it and maybe changed in 0.49. The expected behavior should be confirmed with the Services team.

SimiHunjan commented 1 month ago

@quiet-node the signature requirements come from the node software not the SDKs. If the auto renew account is not required to sign this would need to be changed in Services/consensus node software.

quiet-node commented 1 month ago

Ah nice it makes sense thanks @SimiHunjan! However, I went through the changelog from 2.41 to 2.42 and didn't see anything related to upgrading node-network from mono to mod, or maybe I'm just not familiar with the codebase, and the upgrading work is done somewhere else?

SimiHunjan commented 1 month ago

we don't need to update node version in the SDKs. The SDKs allow you to point to the node or network endpoints you want to submit transactions to. The node versions are updated on the nodes themselves i.e. testnet nodes would update to 0.49. If there are new features added to 0.49 consensus node software there will be an SDK version released that allows you to create and submit any new transactions or queries that were added in 0.49.

The resolution of this issue is to update the code to sign with the auto renew account's private key.

quiet-node commented 1 month ago

The issue was resolved in this pull request. Previously, the accountId set for the auto-renew account was ignored and replaced by the operator account ID, which is why it passed without any exceptions. Thank you, @SimiHunjan and @Neeharika-Sompalli, for pointing it out!