Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Commit

Permalink
Merge pull request #4878 from LiskHQ/4845-remove_tx_validation
Browse files Browse the repository at this point in the history 
Remove transaction independent verify and total spending checks - Closes #4845
  • Loading branch information
@ManuGowda
ManuGowda authored Feb 26, 2020
2 parents 3c1c1ea + 2d7b373 commit 4a5771c
Show file tree
Hide file tree
Showing 9 changed files with 22 additions and 1,379 deletions.
9 changes: 4 additions & 5 deletions elements/lisk-chain/src/chain.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import {
processSignature,
undoTransactions,
validateTransactions,
verifyTransactions,
} from './transactions';
import { TransactionHandledResult } from './transactions/compose_transaction_steps';
import {
Expand Down Expand Up @@ -292,7 +291,6 @@ export class Chain {
this.blocksVerify = new BlocksVerify({
dataAccess: this.dataAccess,
exceptions: this.exceptions,
slots: this.slots,
genesisBlock: this.genesisBlock,
});
}
Expand Down Expand Up @@ -429,7 +427,7 @@ export class Chain {

public async verify(
blockInstance: BlockInstance,
stateStore: StateStore,
_: StateStore,
{ skipExistingCheck }: { readonly skipExistingCheck: boolean },
): Promise<void> {
if (!skipExistingCheck) {
Expand All @@ -447,7 +445,7 @@ export class Chain {
throw invalidPersistedResponse.errors;
}
}
await this.blocksVerify.checkTransactions(blockInstance, stateStore);
await this.blocksVerify.checkTransactions(blockInstance);
}

public async apply(
Expand Down Expand Up @@ -628,6 +626,7 @@ export class Chain {
)(transactions);
}

// TODO: Remove this function in #4841 as it is not needed on the new transaction pool
public async verifyTransactions(
transactions: BaseTransaction[],
): Promise<TransactionHandledResult> {
Expand All @@ -644,7 +643,7 @@ export class Chain {
};
}),
checkPersistedTransactions(this.dataAccess),
verifyTransactions(this.slots, this.exceptions),
applyTransactions(this.exceptions),
)(transactions, stateStore);
}

Expand Down
1 change: 0 additions & 1 deletion elements/lisk-chain/src/transactions/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export {
checkPersistedTransactions,
checkAllowedTransactions,
undoTransactions,
verifyTransactions,
processSignature,
applyGenesisTransactions,
} from './transactions_handlers';
134 changes: 3 additions & 131 deletions elements/lisk-chain/src/transactions/transactions_handlers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
} from '@liskhq/lisk-transactions';

import { DataAccess } from '../data_access';
import { Slots } from '../slots';
import { StateStore } from '../state_store';
import {
Contexter,
Expand Down Expand Up @@ -56,75 +55,6 @@ export const validateTransactions = (exceptions?: ExceptionOptions) => (
};
};

/**
* Verify user total spending
*
* One user can't spend more than its balance even thought if block contains
* credit transactions settling the balance. In one block total speding must be
* less than the total balance
*/
export const verifyTotalSpending = async (
transactions: ReadonlyArray<BaseTransaction>,
stateStore: StateStore,
) => {
const spendingErrors: TransactionResponse[] = [];

// Group the transactions per senderId to calculate total spending
const senderTransactions = transactions.reduce((rv, x) => {
(rv[x.senderId] = rv[x.senderId] || []).push(x);

return rv;
// tslint:disable-next-line readonly-keyword no-object-literal-type-assertion
}, {} as { [key: string]: BaseTransaction[] });

// We need to get the transaction id which cause exceeding the sufficient balance
// So we can't sum up all transactions together at once
// tslint:disable-next-line readonly-keyword
const senderSpending: { [key: string]: bigint } = {};
for (const senderId of Object.keys(senderTransactions)) {
// We don't need to perform spending check if account have only one transaction
// Its balance check will be performed by transaction processing
// tslint:disable-next-line no-magic-numbers
if (senderTransactions[senderId].length < 2) {
// tslint:disable-next-line: return-undefined
continue;
}

// Grab the sender balance
const account = await stateStore.account.get(senderId);
const senderBalance = account.balance;

// Initialize the sender spending with zero
senderSpending[senderId] = BigInt(0);

senderTransactions[senderId].forEach((transaction: BaseTransaction) => {
const senderTotalSpending =
senderSpending[senderId] +
// tslint:disable-next-line no-any
BigInt((transaction.asset as any).amount || 0) +
BigInt(transaction.fee);

if (senderBalance < senderTotalSpending) {
spendingErrors.push({
id: transaction.id,
status: TransactionStatus.FAIL,
errors: [
new TransactionError(
`Account does not have enough LSK for total spending. balance: ${senderBalance.toString()}, spending: ${senderTotalSpending.toString()}`,
transaction.id,
'.amount',
),
],
});
} else {
senderSpending[senderId] = senderTotalSpending;
}
});
}

return spendingErrors;
};

export const applyGenesisTransactions = () => async (
transactions: ReadonlyArray<BaseTransaction>,
stateStore: StateStore,
Expand Down Expand Up @@ -167,28 +97,15 @@ export const applyTransactions = (exceptions?: ExceptionOptions) => async (

await votesWeight.prepare(stateStore, transactions);

// Verify total spending of per account accumulative
const transactionsResponseWithSpendingErrors = await verifyTotalSpending(
transactions,
stateStore,
);

const transactionsWithoutSpendingErrors = transactions.filter(
transaction =>
!transactionsResponseWithSpendingErrors
.map(({ id }) => id)
.includes(transaction.id),
);

const transactionsResponses: TransactionResponse[] = [];
for (const transaction of transactionsWithoutSpendingErrors) {
for (const transaction of transactions) {
stateStore.account.createSnapshot();
const transactionResponse = await transaction.apply(stateStore);
if (transactionResponse.status !== TransactionStatus.OK) {
// Update transaction response mutates the transaction response object
exceptionsHandlers.updateTransactionResponseForExceptionTransactions(
[transactionResponse],
transactionsWithoutSpendingErrors,
transactions,
exceptions,
);
}
Expand All @@ -204,10 +121,7 @@ export const applyTransactions = (exceptions?: ExceptionOptions) => async (
}

return {
transactionsResponses: [
...transactionsResponses,
...transactionsResponseWithSpendingErrors,
],
transactionsResponses: [...transactionsResponses],
};
};

Expand Down Expand Up @@ -314,48 +228,6 @@ export const undoTransactions = (exceptions?: ExceptionOptions) => async (
};
};

export const verifyTransactions = (
slots: Slots,
exceptions?: ExceptionOptions,
) => async (
transactions: ReadonlyArray<BaseTransaction>,
stateStore: StateStore,
): Promise<TransactionHandledResult> => {
await Promise.all(transactions.map(t => t.prepare(stateStore)));
const transactionsResponses = [];
for (const transaction of transactions) {
stateStore.createSnapshot();
const transactionResponse = await transaction.apply(stateStore);
if (slots.getSlotNumber(transaction.timestamp) > slots.getSlotNumber()) {
(transactionResponse as WriteableTransactionResponse).status =
TransactionStatus.FAIL;
(transactionResponse.errors as TransactionError[]).push(
new TransactionError(
'Invalid transaction timestamp. Timestamp is in the future',
transaction.id,
'.timestamp',
),
);
}
stateStore.restoreSnapshot();
transactionsResponses.push(transactionResponse);
}

const unverifiableTransactionsResponse = transactionsResponses.filter(
transactionResponse => transactionResponse.status !== TransactionStatus.OK,
);

exceptionsHandlers.updateTransactionResponseForExceptionTransactions(
unverifiableTransactionsResponse,
transactions,
exceptions,
);

return {
transactionsResponses,
};
};

export const processSignature = () => async (
transaction: BaseTransaction,
signature: SignatureObject,
Expand Down
27 changes: 1 addition & 26 deletions elements/lisk-chain/src/verify.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ import {
} from '@liskhq/lisk-transactions';

import { DataAccess } from './data_access';
import { Slots } from './slots';
import { StateStore } from './state_store';
import * as transactionsModule from './transactions';
import {
BlockHeader,
Expand Down Expand Up @@ -63,25 +61,21 @@ export const verifyPreviousBlockId = (

interface BlockVerifyInput {
readonly dataAccess: DataAccess;
readonly slots: Slots;
readonly exceptions: ExceptionOptions;
readonly genesisBlock: BlockHeader;
}

export class BlocksVerify {
private readonly dataAccess: DataAccess;
private readonly slots: Slots;
private readonly exceptions: ExceptionOptions;
private readonly genesisBlock: BlockHeader;

public constructor({
dataAccess,
exceptions,
slots,
genesisBlock,
}: BlockVerifyInput) {
this.dataAccess = dataAccess;
this.slots = slots;
this.exceptions = exceptions;
this.genesisBlock = genesisBlock;
}
Expand All @@ -108,10 +102,7 @@ export class BlocksVerify {
}
}

public async checkTransactions(
blockInstance: BlockInstance,
stateStore: StateStore,
): Promise<void> {
public async checkTransactions(blockInstance: BlockInstance): Promise<void> {
const { version, height, timestamp, transactions } = blockInstance;
if (transactions.length === 0) {
return;
Expand Down Expand Up @@ -142,22 +133,6 @@ export class BlocksVerify {
if (nonAllowedTxResponses) {
throw nonAllowedTxResponses.errors;
}

const {
transactionsResponses,
} = await transactionsModule.verifyTransactions(
this.slots,
this.exceptions,
)(nonInertTransactions, stateStore);

const unverifiableTransactionsResponse = transactionsResponses.filter(
(transactionResponse: TransactionResponse) =>
transactionResponse.status !== TransactionStatus.OK,
);

if (unverifiableTransactionsResponse.length > 0) {
throw unverifiableTransactionsResponse[0].errors;
}
}

public matchGenesisBlock(block: BlockHeader): boolean {
Expand Down
43 changes: 4 additions & 39 deletions elements/lisk-chain/test/unit/process.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('blocks/header', () => {

describe('#validateBlockHeader', () => {
describe('when previous block property is invalid', () => {
it('should throw error', () => {
it('should throw error', async () => {
// Arrange
block = newBlock({ previousBlockId: undefined, height: 3 });
blockBytes = getBytes(block);
Expand All @@ -128,6 +128,7 @@ describe('blocks/header', () => {
).toThrow('Invalid previous block');
});
});

describe('when signature is invalid', () => {
it('should throw error', async () => {
// Arrange
Expand Down Expand Up @@ -282,7 +283,7 @@ describe('blocks/header', () => {
// Act & assert
expect(() =>
chainInstance.validateBlockHeader(block, blockBytes, defaultReward),
).toResolve();
).not.toThrow();
});
});
});
Expand Down Expand Up @@ -446,43 +447,6 @@ describe('blocks/header', () => {
});
});

describe('when skip existing check is true and a transaction is not verifiable', () => {
let invalidTx;

beforeEach(async () => {
// Arrage
storageStub.entities.Account.get.mockResolvedValue([
{ address: genesisAccount.address, balance: '0' },
]);
invalidTx = chainInstance.deserializeTransaction(
transfer({
passphrase: genesisAccount.passphrase,
recipientId: '123L',
amount: '100',
networkIdentifier,
}) as TransactionJSON,
);
block = newBlock({ transactions: [invalidTx] });
});

it('should not call apply for the transaction and throw error', async () => {
// Act
const stateStore = new StateStore(storageStub);

await expect(
chainInstance.verify(block, stateStore, {
skipExistingCheck: true,
}),
).rejects.toMatchObject([
expect.objectContaining({
message: expect.stringContaining(
'Account does not have enough LSK',
),
}),
]);
});
});

describe('when skip existing check is true and transactions are valid', () => {
let invalidTx;

Expand All @@ -505,6 +469,7 @@ describe('blocks/header', () => {
it('should not call apply for the transaction and throw error', async () => {
// Act
const stateStore = new StateStore(storageStub);
expect.assertions(1);
let err;
try {
await chainInstance.verify(block, stateStore, {
Expand Down
Loading

0 comments on commit 4a5771c

Please sign in to comment.