hashgraph / hedera-sdk-js

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

FileAppend serialize and deserialize breaks freezing #2536

Closed SvetBorislavov closed 1 month ago

SvetBorislavov commented 2 months ago

Description

When you have a File Append Transaction and want to send it somewhere unfrozen, you will serialize it and deserialize it afterward. Then when you try to freeze it, it won't get frozen and the transaction id will be lost.

Maybe the issue can be connected with: #2167

Steps to reproduce

  1. Run the following snippet:
    
    /* imports */
    const {
    Client,
    AccountId,
    Transaction,
    FileAppendTransaction,
    TransactionId,
    Timestamp,
    } = require('@hashgraph/sdk');

async function run() { const accountId = AccountId.fromString('0.0.2159149'); const client = Client.forTestnet();

// Create a File Append Transaction let transaction = new FileAppendTransaction().setTransactionId( TransactionId.withValidStart(accountId, Timestamp.fromDate(new Date())) );

// Serialize const transactionBytes = transaction.toBytes();

// Deserialize transaction = Transaction.fromBytes(transactionBytes);

// Freeze transaction = transaction.freezeWith(client);

// Verify the bug console.log(transaction.isFrozen() === false); console.log(transaction.transactionId === null);

client.close(); }

run();



### Additional context

_No response_

### Hedera network

_No response_

### Version

2.51.0

### Operating system

None
ivaylonikolov7 commented 2 months ago

According to HIP-745 freeze should work like what you experienced.

Freezing a transaction in a dApp merely sets a flag against the transaction object, the network is not aware of the transaction at this point. Developers are sometimes confused, believing that if the transaction is frozen, it can’t ever be modified, but this isn’t strictly true. A wallet (or anyone receiving the serialised transaction) can deserialise the transaction and create a new one from the serialised data, this new transaction will be unfrozen and subject to being modified by the wallet. When the purpose of freezing a transaction is misunderstood, it leads to a false sense of security.

Keeping the transactionId of the transaction is tricky. The implementation of the chunked transactions is not the same as the standard transaction. When you call toBytes of transaction with chunks in the SDK, you create an empty transaction list and populate it with n chunk transaction. We generate the real transaction IDs in that transaction list. The one of the original "main" transaction is mostly used like a dummy transaction id. You can also get the transactionId from the receipt when you execute the transaction. I think this is implemented the same way across all the SDKs and it's not just a JS SDK issue.

Will this be a blocker for the transaction tool team? If so I will talk with our team on how can we resolve this.

SvetBorislavov commented 2 months ago

If we deserialize a transaction, it will not be frozen, which is okay. The problem is that the freezeWith method is not freezing the transaction. As you can see in the snippet, the transaction is frozen after the deserialization but when I check if it is frozen, false is returned. This blocks the signing functionality.

For the transaction id, I understand, it is empty, because I don't set any content so no chunks are created.

SvetBorislavov commented 2 months ago

It is a blocker for our functionality as we cannot use the File Append, which is used together with File Create and File Update.

agadzhalov commented 1 month ago

fixed in https://github.com/hashgraph/hedera-sdk-js/pull/2532