Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct return signed vs unsigned serialized data #177

Merged
merged 3 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 6 additions & 15 deletions src/transaction/abstract-transaction.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Signature } from '../crypto/index.js';
import { keccak256, Signature } from '../crypto/index.js';
import { getBigInt, assert, assertArgument } from '../utils/index.js';

import type { BigNumberish } from '../utils/index.js';
Expand Down Expand Up @@ -281,8 +281,8 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
*
* This is the digest that a [Signer](../interfaces/Signer) must sign to authorize this transaction.
*/
get unsignedHash(): string {
throw new Error('Method not implemented.');
get digest(): string {
return keccak256(this.unsignedSerialized);
}

/**
Expand Down Expand Up @@ -315,7 +315,7 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
'UNSUPPORTED_OPERATION',
{ operation: '.serialized' },
);
return this.#serialize();
return encodeProtoTransaction(this.toProtobuf(true));
}

/**
Expand All @@ -324,7 +324,7 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
* The hash of this is the digest which needs to be signed to authorize this transaction.
*/
get unsignedSerialized(): string {
return this.#serialize();
return encodeProtoTransaction(this.toProtobuf(false));
}

/**
Expand Down Expand Up @@ -362,7 +362,7 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
*
* @returns {ProtoTransaction} The protobuf-friendly JSON object.
*/
abstract toProtobuf(): ProtoTransaction;
abstract toProtobuf(includeSignature: boolean): ProtoTransaction;

abstract get originZone(): Zone | undefined;

Expand All @@ -371,13 +371,4 @@ export abstract class AbstractTransaction<S extends allowedSignatureTypes> imple
get isExternal(): boolean {
return this.destZone !== undefined && this.originZone !== this.destZone;
}

/**
* Serializes the WorkObject to a string.
*
* @returns {string} The serialized WorkObject.
*/
#serialize(): string {
return encodeProtoTransaction(this.toProtobuf());
}
}
42 changes: 22 additions & 20 deletions src/transaction/qi-transaction.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
import {keccak256} from "../crypto/index.js";
import {AbstractTransaction, TransactionLike, TxInput, TxOutput} from "./index.js";
import {
assertArgument,
getBytes, getZoneForAddress,
hexlify, isQiAddress,
toBigInt
} from "../utils/index.js";
import { keccak256 } from '../crypto/index.js';
import { AbstractTransaction, TransactionLike, TxInput, TxOutput } from './index.js';
import { assertArgument, getBytes, getZoneForAddress, hexlify, isQiAddress, toBigInt } from '../utils/index.js';
import { decodeProtoTransaction } from '../encoding/index.js';
import {formatNumber} from "../providers/format.js";
import { computeAddress } from "../address/index.js";
import { ProtoTransaction} from "./abstract-transaction.js";
import { formatNumber } from '../providers/format.js';
import { computeAddress } from '../address/index.js';
import { ProtoTransaction } from './abstract-transaction.js';
import { Zone } from '../constants/index.js';

/**
Expand Down Expand Up @@ -57,28 +52,30 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
this.#txOutputs = value.map((output) => ({ ...output }));
}

/**
* The permuted hash of the transaction as specified by
* [QIP-0010](https://github.com/quai-network/qips/blob/master/qip-0010.md).
*/
get hash(): null | string {
if (this.signature == null) {
return null;
}
return this.unsignedHash;
}
get unsignedHash(): string {

if (this.txInputs.length < 1 || this.txOutputs.length < 1) {
throw new Error('Transaction must have at least one input and one output');
}

const destUtxo = isQiAddress(hexlify(this.txOutputs[0].address) || '');
const pubKey = hexlify(this.txInputs[0].pub_key);
const senderAddr = computeAddress(pubKey || '');
const originUtxo = isQiAddress(senderAddr);

if (!this.destZone || !this.originZone) {
throw new Error(
`Invalid zones: origin ${this.originZone} -> destination ${this.destZone} (address: ${senderAddr})`,
);
}
if (this.isExternal && destUtxo !== originUtxo) {

const isSameLedger = isQiAddress(senderAddr) === isQiAddress(hexlify(this.txOutputs[0].address) || '');
if (this.isExternal && !isSameLedger) {
throw new Error('Cross-zone & cross-ledger transactions are not supported');
}

Expand All @@ -97,6 +94,9 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
return '0x' + hashBuffer.toString('hex');
}

/**
* The zone of the sender address
*/
get originZone(): Zone | undefined {
const pubKey = hexlify(this.txInputs[0].pub_key);
const senderAddr = computeAddress(pubKey || '');
Expand All @@ -105,6 +105,9 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
return zone ?? undefined;
}

/**
* The zone of the recipient address
*/
get destZone(): Zone | undefined {
const zone = getZoneForAddress(hexlify(this.txOutputs[0].address) || '');
return zone ?? undefined;
Expand Down Expand Up @@ -176,15 +179,15 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
*
* @returns {ProtoTransaction} The protobuf-friendly JSON object.
*/
toProtobuf(): ProtoTransaction {
toProtobuf(includeSignature: boolean = true): ProtoTransaction {
const protoTx: ProtoTransaction = {
type: this.type || 2,
chain_id: formatNumber(this.chainId || 0, 'chainId'),
tx_ins: { tx_ins: this.txInputs },
tx_outs: { tx_outs: this.txOutputs },
};

if (this.signature) {
if (this.signature && includeSignature) {
protoTx.signature = getBytes(this.signature);
}

Expand Down Expand Up @@ -237,7 +240,6 @@ export class QiTransaction extends AbstractTransaction<string> implements QiTran
* @returns {QiTransaction} The decoded transaction.
*/
static fromProto(protoTx: ProtoTransaction): QiTransaction {
// TODO: Fix this because new tx instance requires a 'from' address
const tx = new QiTransaction();

tx.type = protoTx.type;
Expand Down
37 changes: 19 additions & 18 deletions src/transaction/quai-transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
zeroPadValue,
} from '../utils/index.js';
import { decodeProtoTransaction, encodeProtoTransaction } from '../encoding/index.js';
import { recoverAddress, validateAddress } from '../address/index.js';
import { getAddress, recoverAddress, validateAddress } from '../address/index.js';
import { formatNumber, handleNumber } from '../providers/format.js';
import { ProtoTransaction } from './abstract-transaction.js';
import { Zone } from '../constants';
Expand Down Expand Up @@ -120,21 +120,19 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
this.#to = value;
}

/**
* The permuted hash of the transaction as specified by
* [QIP-0010](https://github.com/quai-network/qips/blob/master/qip-0010.md).
*/
get hash(): null | string {
if (this.signature == null) {
return null;
}
return this.unsignedHash;
}

get unsignedHash(): string {
const destUtxo = isQiAddress(this.to || '');
const originUtxo = isQiAddress(this.from);
if (this.signature == null) return null;

if (!this.originZone) {
throw new Error('Invalid Zone for from address');
}
if (this.isExternal && destUtxo !== originUtxo) {

const isSameLedger = isQiAddress(this.from) === isQiAddress(this.to || '');
if (this.isExternal && !isSameLedger) {
throw new Error('Cross-zone & cross-ledger transactions are not supported');
}

Expand All @@ -153,13 +151,17 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
return '0x' + hashBuffer.toString('hex');
}

/**
* The zone of the sender address
*/
get originZone(): Zone | undefined {
const senderAddr = this.from;

const zone = getZoneForAddress(senderAddr);
const zone = getZoneForAddress(this.from);
return zone ?? undefined;
}

/**
* The zone of the recipient address
*/
get destZone(): Zone | undefined {
const zone = this.to !== null ? getZoneForAddress(this.to || '') : undefined;
return zone ?? undefined;
Expand Down Expand Up @@ -361,7 +363,7 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
*
* @returns {ProtoTransaction} The protobuf-friendly JSON object.
*/
toProtobuf(): ProtoTransaction {
toProtobuf(includeSignature: boolean = true): ProtoTransaction {
const protoTx: ProtoTransaction = {
type: this.type || 0,
chain_id: formatNumber(this.chainId || 0, 'chainId'),
Expand All @@ -375,7 +377,7 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q
access_list: { access_tuples: [] },
};

if (this.signature) {
if (this.signature && includeSignature) {
protoTx.v = formatNumber(this.signature.yParity, 'yParity');
protoTx.r = toBeArray(this.signature.r);
protoTx.s = toBeArray(this.signature.s);
Expand Down Expand Up @@ -484,8 +486,7 @@ export class QuaiTransaction extends AbstractTransaction<Signature> implements Q

if (protoTx.to !== null) {
const toAddr = hexlify(protoTx.to!);
validateAddress(toAddr);
tx.to = toAddr;
tx.to = getAddress(toAddr);
}

tx.type = protoTx.type;
Expand Down
Loading