hashgraph / hedera-sdk-js

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

Multi node, multi signature offline signing support #2481

Open jbair06 opened 3 weeks ago

jbair06 commented 3 weeks ago

Problem

The SDK currently does not support the use case of multiple signatures for multiple nodes when signing is done offline. Transaction.sign(key) works for multi node, multi sig but not offline. PrivateKey.signTransaction(transaction) supports offline multi sig, but not multi node.

Solution

A new object and a couple new methods can solve this situation.

Alternatives

No response

jbair06 commented 3 weeks ago

After some research, I discovered this to be an interesting problem. As chunked transactions is possible, the 'SignatureDictionary' isn't quite enough to make it work for all cases. The overall concept could still be viable, but might need more work. Perhaps if the 'Signature' interface is a dictionary of dictionaries? { [transactionId: string]: { [nodeAccountId: string]: Uint8Array } }

jbair06 commented 2 weeks ago

I've worked on this a bit and have a proposed solution. @ivaylonikolov7 and @agadzhalov, here are the code snippets you requested.

jbair06 commented 2 weeks ago

PrivateKey.js

    /**
     * Sign a message with this private key.
     *
     * @param {Uint8Array | Transaction} data - The data to be signed. Can be of type Uint8Array or Transaction.
     * @returns {Uint8Array | Uint8Array[]} - The signature bytes without the message
     */
    sign(data) {
        if (data instanceof Uint8Array) {
            // If data is Uint8Array, sign it directly
            return this._key.sign(data);
        } else if (data instanceof Transaction) {
            // If data is a Transaction, sign the full transaction (including all nodes and chunks)
            const signature = []
            for (const signedTransaction of data._signedTransactions.list) {
                const bodyBytes = /** @type {Uint8Array} */ (
                    signedTransaction.bodyBytes
                );
                signature.push(this._key.sign(bodyBytes));
            }
            return signature;
        } else {
            throw new Error("Invalid argument. Expected Uint8Array or Transaction.");
        }
    }
jbair06 commented 2 weeks ago

Transaction.js

    /**
     * Add a signature explicitly
     *
     * This method requires the transaction to have exactly 1 node account ID set
     * since different node account IDs have different byte representations and
     * hence the same signature would not work for all transactions that are the same
     * except for node account ID being different.
     *
     * @param {PublicKey} publicKey
     * @param {Uint8Array | Unit8Array[]} signature
     * @returns {this}
     */
    addSignature(publicKey, signature) {
        const signatureArray = [...signature];
        // If the signature is a Uint8Array, require that only one node is set on this transaction
        // Otherwise ensure that the signature array is the same length as the number of transactions
        if (signature instanceof Uint8Array) {
            this._requireOneNodeAccountId();
        } else if (signature.length !== this._signedTransactions.length) {
            throw new Error(
                "signature array must be the same length as the number of transactions",
            );
        }

        // If the transaction isn't frozen, freeze it.
        if (!this.isFrozen()) {
            this.freeze();
        }

        const publicKeyData = publicKey.toBytesRaw();
        const publicKeyHex = hex.encode(publicKeyData);

        if (this._signerPublicKeys.has(publicKeyHex)) {
            // this public key has already signed this transaction
            return this;
        }

        // If we add a new signer, then we need to re-create all transactions
        this._transactions.clear();

        // Locking the transaction IDs and node account IDs is necessary for consistency
        // between before and after execution
        this._transactionIds.setLocked();
        this._nodeAccountIds.setLocked();
        this._signedTransactions.setLocked();

        // Add the signature to the signed transaction list.
        for (let index = 0; index < this._signedTransactions.length; index++) {
            const signedTransaction = this._signedTransactions.get(index);

            if (signedTransaction.sigMap == null) {
                signedTransaction.sigMap = {};
            }

            if (signedTransaction.sigMap.sigPair == null) {
                signedTransaction.sigMap.sigPair = [];
            }

            signedTransaction.sigMap.sigPair.push(
                publicKey._toProtobufSignature(signatureArray[index]),
            );
        }

        this._signerPublicKeys.add(publicKeyHex);
        this._publicKeys.push(publicKey);
        this._transactionSigners.push(null);

        return this;
    }
jbair06 commented 2 weeks ago

PublicKey.js

    /**
     * Verify a signature on a message with this public key.
     *
     * @param {Uint8Array | Transaction} message
     * @param {Uint8Array | Uint8Array[]} signature
     * @returns {boolean}
     */
    verify(message, signature) {
        if (message instanceof Uint8Array && signature instanceof Uint8Array) {
            return this._key.verify(message, signature);
        } else if (message instanceof Transaction && Array.isArray(signature)) {
            for (let index = 0; index < message._signedTransactions.length; index++) {
                const signedTranasction = message._signedTransactions[index];
                return this._key.verify(signedTranasaction.bodyBytes, signature[index]);
            }
        }
        else {
            throw new Error(
                "Invalid arguments, expected either (Uint8Array, Uint8Array) or (Transaction, Uint8Array)");
        }
    }
jbair06 commented 2 weeks ago

I did not update all the comments/documentation. My main concern with the proposed solution is that the signature array returned in PrivateKey.sign may not be guaranteed to be in the same order as the transaction's internal signedTransaction list, I think. But I don't fully understand when the internal lists get locked.

Which brings up the point we discussed offline: I don't understand the Transaction._fromProtobufTransactions function. It doesn't appear to check for any flags or anything in order to determine if those lists should be locked after being set. But maybe it doesn't need to?