hedera-sdk-js icon indicating copy to clipboard operation
hedera-sdk-js copied to clipboard

Improve transaction NodeAccountIdSignatureMap

Open Svetoslavb04 opened this issue 4 months ago • 0 comments

Problem

Currently, the transaction SignatureMap is and ObjectMap<AccountId, NodeAccountIdSignatureMap>, where AccountId is the Node Account Id and the NodeAccountIdSignatureMap is the structure holding a map of ObjectMap<PublicKey, Uint8Array> or just the signature pair of public key and signature.

To be able to search for a specific transaction's signature, you should search in the map as follows: node account id --> public key

That works fine when the _signedTransactions has just one transaction per node, but the _signedTransactions is a 2-D array built into one, which supports more than one transaction per node.

The search in the array is done using that index formula: column * rowLength + row (there is a mistake in the _signedTransactions description or the row and column should be swapped)

  • The row is the node account id
  • The rowLength is the number of node account ids
  • The column is the transaction

When there are two or more transactions in the _signedTransactions, the SignatureMap and the NodeAccountIdSignatureMap fail to return a structure that includes the signatures for all transactions. This is because the NodeAccountIdSignatureMap stores only a public key and a signature. It should be and is actually a SignaturePairMap; see the proto.ISignaturePair. With the current implementation SignatureMap, loops through the _nodeAccountIds and sets the signatures only for the first row of the _signedTransactions aka the first transaction, if the _signedTransactions has more rows, they are ignored.

A new SignaturePairMap should be created, which will be the current NodeAccountIdSignatureMap and the NodeAccountIdSignatureMap should be changed to an ObjectMap<TransactionId, SignaturePairMap>. If you think that using the TransactionId may be problematic if missing, you shouldn't because the TransactionId will always be set inside the _signedTransactions because:

1. A transaction is determined frozen if `_signedTransactions.length > 0` and to freeze it, it has to have a `transaction id`
2. The `_signedTransactions` is only set inside `_buildSignedTransactions`
3. `_buildSignedTransactions` is invoked in 2 places:
   3.1 `freezeWith`: to invoke a `freezeWith`, the transaction id should be set
   3.2 `_buildAllTransactionsAsync`: `_buildSignedTransactions` is invoked only if the `_signOnDemand` is true, and it can be true only when the transaction is frozen

The above proves that the transaction id will be available in the bodyBytes inside the _signedTransactions to be used as a key of the NodeAccountIdSignatureMap.

Here is a solution for these maps (all inside the transaction's folder), feel free to step on them: SignaturePairMap:

import ObjectMap from "../ObjectMap.js";
import PublicKey from "../PublicKey.js";

/**
 * @augments {ObjectMap<PublicKey, Uint8Array>}
 */
export default class SignaturePairMap extends ObjectMap {
    constructor() {
        super((s) => PublicKey.fromString(s));
    }

    /**
     * @param {import("@hashgraph/proto").proto.ISignatureMap} sigMap
     * @returns {SignaturePairMap}
     */
    static _fromTransactionSigMap(sigMap) {
        const signatures = new SignaturePairMap();

        const sigPairs = sigMap.sigPair != null ? sigMap.sigPair : [];

        for (const sigPair of sigPairs) {
            if (sigPair.pubKeyPrefix != null) {
                if (sigPair.ed25519 != null) {
                    signatures._set(
                        PublicKey.fromBytesED25519(sigPair.pubKeyPrefix),
                        sigPair.ed25519,
                    );
                } else if (sigPair.ECDSASecp256k1 != null) {
                    signatures._set(
                        PublicKey.fromBytesECDSA(sigPair.pubKeyPrefix),
                        sigPair.ECDSASecp256k1,
                    );
                }
            }
        }

        return signatures;
    }
}

NodeAccountIdSignatureMap:


import ObjectMap from "../ObjectMap.js";
import TransactionId from "./TransactionId.js";
import SignaturePairMap from "./SignaturePairMap.js";
import * as HashgraphProto from "@hashgraph/proto";

/**
 * @augments {ObjectMap<TransactionId, SignaturePairMap>}
 */
export default class NodeAccountIdSignatureMap extends ObjectMap {
    constructor() {
        super((s) => TransactionId.fromString(s));
    }

    /**
     * @param { import('./List.js').default<import("@hashgraph/proto").proto.ISignedTransaction>} signedTransactions
     * @returns {NodeAccountIdSignatureMap}
     */
    static _fromSignedTransactions(signedTransactions) {
        const signatures = new NodeAccountIdSignatureMap();

        for (const { bodyBytes, sigMap } of signedTransactions.list) {
            if (bodyBytes != null && sigMap != null) {
                const body =
                    HashgraphProto.proto.TransactionBody.decode(bodyBytes);

                if (body.transactionID != null) {
                    const transactionId = TransactionId._fromProtobuf(
                        body.transactionID,
                    );

                    signatures._set(
                        transactionId,
                        SignaturePairMap._fromTransactionSigMap(sigMap),
                    );
                }
            }
        }

        return signatures;
    }
}

SignatureMap

export default class SignatureMap extends ObjectMap {
    constructor() {
        super((s) => AccountId.fromString(s));
    }

    /**
     * @param {import("./Transaction.js").default} transaction
     * @returns {SignatureMap}
     */
    static _fromTransaction(transaction) {
        const signatures = new SignatureMap();

        const rowLength = transaction._nodeAccountIds.length;
        const columns = transaction._signedTransactions.length / rowLength;

        for (let row = 0; row < rowLength; row++) {
            /** @type { List<import("@hashgraph/proto").proto.ISignedTransaction> } */
            const signedTransactions = new List();

            for (let col = 0; col < columns; col++) {
                signedTransactions.push(
                    transaction._signedTransactions.get(col * rowLength + row),
                );
            }

            signatures._set(
                transaction._nodeAccountIds.list[row],
                NodeAccountIdSignatureMap._fromSignedTransactions(
                    signedTransactions,
                ),
            );
        }

        return signatures;
    }
}

Now since the SignatureMap is updated to include node account id, transaction id and public key, it can be accessed this way: node account id --> transaction id --> public key

This update gives a chance for the addSignature function to accept such signature map and add the signatures by searching for the correct transaction by node account id and transaction id. This will prevent breaking the integrity of the _signedTransactions. This update of the SignatureMap structure will require revisiting other functions such as the removeSignature.

Solution

In the scope of Transaction

  • Create SignaturePairMap
  • Update NodeAccountIdSignatureMap
  • Update SignatureMap
  • Update addSignature
  • Update removeSignature
  • Update all relevant functionality that relied on previous structure of SignatureMap

Alternatives

No response

Svetoslavb04 avatar Oct 18 '24 13:10 Svetoslavb04