Skip to content

Commit

Permalink
Merge branch '399-fix-redundant-tx-generation-in-cold-storage-process…
Browse files Browse the repository at this point in the history 
…' into 'dev'

refactor cold storage to support parallel txs

Closes #399

See merge request ergo/rosen-bridge/guard-service!384
  • Loading branch information
@zargarzadehm
zargarzadehm committed Aug 29, 2024
2 parents 0a86b77 + 1a2d193 commit f8ab88b
Show file tree
Hide file tree
Showing 8 changed files with 369 additions and 287 deletions.
5 changes: 5 additions & 0 deletions .changeset/proud-otters-joke.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'guard-service': minor
---

refactor cold storage tx verification and its insertion handler to support parallel cold storage txs
32 changes: 4 additions & 28 deletions src/db/DatabaseHandler.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,35 +121,11 @@ class DatabaseHandler {
await DatabaseAction.getInstance().getActiveColdStorageTxsInChain(
newTx.network
);
if (txs.length > 1) {
throw new ImpossibleBehavior(
`Chain [${newTx.network}] has already more than 1 (${txs.length}) active cold storage tx`
if (txs.some((tx) => tx.txId === newTx.txId)) {
logger.info(
`Reinsertion for cold storage tx [${newTx.txId}], 'failedInSign' updated to false`
);
} else if (txs.length === 1) {
const tx = txs[0];
if (tx.txId === newTx.txId) {
logger.info(
`Reinsertion for cold storage tx [${tx.txId}], 'failedInSign' updated to false`
);
await DatabaseAction.getInstance().resetFailedInSign(tx.txId);
} else {
if (tx.status === TransactionStatus.approved) {
if (newTx.txId < tx.txId) {
logger.info(
`Replacing cold storage tx [${tx.txId}] with new transaction [${newTx.txId}] due to lower txId`
);
await DatabaseAction.getInstance().replaceTx(tx.txId, newTx);
} else
logger.info(
`Ignoring new cold storage tx [${newTx.txId}] due to higher txId, comparing to [${tx.txId}]`
);
} else {
if (newTx.txId !== tx.txId)
logger.warn(
`Received approval for new tx [${newTx.txId}] where its chain [${newTx.network}] has already in progress tx [${tx.txId}]`
);
}
}
await DatabaseAction.getInstance().resetFailedInSign(newTx.txId);
} else
await DatabaseAction.getInstance().insertNewTx(newTx, null, requiredSign);
};
Expand Down
17 changes: 0 additions & 17 deletions src/verification/RequestVerifier.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ class RequestVerifier {
/**
* verifies the cold storage transaction request sent by other guards
* conditions:
* - there is no active cold storage transaction
* - requested tx is not malicious
* @param tx the created payment transaction
* @returns true if conditions are met
Expand All @@ -100,22 +99,6 @@ class RequestVerifier {
): Promise<boolean> => {
const baseError = `Received cold storage tx [${tx.txId}] `;

// check if event has any active tx for requested tx type
const inProgressColdStorageTxs =
await DatabaseAction.getInstance().getActiveColdStorageTxsInChain(
tx.network
);
if (
inProgressColdStorageTxs.length !== 0 &&
inProgressColdStorageTxs[0].txId !== tx.txId
) {
logger.warn(
baseError +
`but found active cold storage tx [${inProgressColdStorageTxs[0].txId}]`
);
return false;
}

// verify requested tx
if (!(await TransactionVerifier.verifyColdStorageTransaction(tx))) {
logger.warn(baseError + `but tx hasn't verified`);
Expand Down
42 changes: 39 additions & 3 deletions src/verification/TransactionVerifier.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { JsonBI } from '../network/NetworkModels';
import { DatabaseAction } from '../db/DatabaseAction';
import WinstonLogger from '@rosen-bridge/winston-logger';
import { ChainNativeToken } from '../utils/constants';
import * as TransactionSerializer from '../transaction/TransactionSerializer';

const logger = WinstonLogger.getInstance().getLogger(import.meta.url);

Expand Down Expand Up @@ -117,9 +118,10 @@ class TransactionVerifier {
* verifies the cold storage transaction
* conditions:
* - tx order is to cold storage address
* - at least one asset required transfer
* - at least one asset requires transfer
* - no forbidden asset is transferred
* - address remaining assets are more than low threshold
* - no active cold storage tx exist that is transferring the same token
* - transferring assets remain more than low threshold in the lock address
* @param tx the created payment transaction
* @returns true if conditions are met
*/
Expand Down Expand Up @@ -159,13 +161,47 @@ class TransactionVerifier {
const nativeTokenId = ChainNativeToken[tx.network];
const isNativeTokenForbade = forbiddenTokens.includes(nativeTokenId);

// no active cold storage tx exist that is transferring the same token
const thresholdsConfig = Configs.thresholds()[tx.network];
const transferringTokenIds = txOrder[0].assets.tokens.map(
(token) => token.id
);
const inProgressColdStorageTxs =
await DatabaseAction.getInstance().getActiveColdStorageTxsInChain(
tx.network
);
for (const activeTx of inProgressColdStorageTxs) {
if (activeTx.txId === tx.txId) continue;
const activeTxOrder = chain.extractTransactionOrder(
TransactionSerializer.fromJson(activeTx.txJson)
);
if (
activeTxOrder[0].assets.tokens.some((token) =>
transferringTokenIds.includes(token.id)
)
) {
logger.debug(
`Transaction [${tx.txId}] is invalid: Tx is transferring a token that is in transfer by tx [${activeTx.txId}]`
);
return false;
}
if (
txOrder[0].assets.nativeToken > thresholdsConfig.maxNativeTransfer &&
activeTxOrder[0].assets.nativeToken > thresholdsConfig.maxNativeTransfer
) {
logger.debug(
`Transaction [${tx.txId}] is invalid: Tx is transferring native token while it is also in transfer by tx [${activeTx.txId}]`
);
return false;
}
}

// verify address assets
const lockedAssets = await chain.getLockAddressAssets();
const remainingAssets = ChainUtils.subtractAssetBalance(
lockedAssets,
txOrder[0].assets
);
const thresholdsConfig = Configs.thresholds()[tx.network];
const thresholds = thresholdsConfig.tokens;

// verify transfer conditions for tokens
Expand Down
191 changes: 2 additions & 189 deletions tests/db/DatabaseHandler.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ describe('DatabaseHandler', () => {
describe('insertColdStorageTx', () => {
/**
* @target DatabaseHandler.insertColdStorageTx should insert tx when
* there is no other tx for the chain
* tx is not in database
* @dependencies
* - database
* @scenario
Expand All @@ -310,7 +310,7 @@ describe('DatabaseHandler', () => {
* @expected
* - tx should be inserted into db
*/
it('should insert tx when there is no other tx for the chain', async () => {
it('should insert tx when tx is not in database', async () => {
// mock transaction
const chain = 'chain';
const tx = TxTestData.mockPaymentTransaction(
Expand All @@ -331,193 +331,6 @@ describe('DatabaseHandler', () => {
expect(dbTxs).toEqual([[tx.txId, null, tx.network]]);
});

/**
* @target DatabaseHandler.insertColdStorageTx should insert tx when
* there is invalid tx for the chain
* @dependencies
* - database
* @scenario
* - mock transaction
* - run test (call `insertTx`)
* - check database
* @expected
* - tx should be inserted into db
*/
it('should insert tx when there is invalid tx for the chain', async () => {
// mock transaction
const chain = 'chain';
const tx = TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
);
const tx2 = TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
);

// insert one of the txs into db
await DatabaseHandlerMock.insertTxRecord(tx2, TransactionStatus.invalid);

// run test
await DatabaseHandler.insertTx(tx, requiredSign);

// tx should be inserted into db
const dbTxs = (await DatabaseHandlerMock.allTxRecords())
.filter((tx) => tx.txId !== tx2.txId)
.map((tx) => [tx.txId, tx.event, tx.chain]);
expect(dbTxs).toEqual([[tx.txId, null, tx.network]]);
});

/**
* @target DatabaseHandler.insertColdStorageTx should NOT insert tx when
* there is already an advanced tx for the chain
* @dependencies
* - database
* @scenario
* - mock two transactions
* - insert one of the txs into db (with `inSign` status)
* - run test (call `insertTx`)
* - check database
* @expected
* - tx should NOT be inserted into db
*/
it('should NOT insert tx when there is already an advanced tx for the chain', async () => {
// mock two transactions
const chain = 'chain';
const tx1 = TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
);
const tx2 = TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
);

// insert one of the txs into db
await DatabaseHandlerMock.insertTxRecord(tx2, TransactionStatus.inSign);

// run test
await DatabaseHandler.insertTx(tx1, requiredSign);

// tx should NOT be inserted into db
const dbTxs = (await DatabaseHandlerMock.allTxRecords()).map(
(tx) => tx.txId
);
expect(dbTxs).toEqual([tx2.txId]);
});

/**
* @target DatabaseHandler.insertColdStorageTx should insert tx when
* txId is lower than existing approved tx
* @dependencies
* - database
* @scenario
* - mock two transactions
* - insert tx with higher txId (with `approved` status)
* - run test (call `insertTx`)
* - check database
* @expected
* - tx should be inserted into db
*/
it('should insert tx when txId is lower than existing approved tx', async () => {
// mock two transactions
const chain = 'chain';
const txs = [
TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
),
TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
),
];
let highTx: PaymentTransaction;
let lowTx: PaymentTransaction;
if (txs[0].txId < txs[1].txId) {
highTx = txs[1];
lowTx = txs[0];
} else {
highTx = txs[0];
lowTx = txs[1];
}

// insert tx with higher txId (with `approved` status)
await DatabaseHandlerMock.insertTxRecord(
highTx,
TransactionStatus.approved
);

// run test
await DatabaseHandler.insertTx(lowTx, requiredSign);

// tx should be inserted into db
const dbTxs = (await DatabaseHandlerMock.allTxRecords()).map(
(tx) => tx.txId
);
expect(dbTxs).toEqual([lowTx.txId]);
});

/**
* @target DatabaseHandler.insertColdStorageTx should NOT insert tx when
* txId is higher than existing approved tx
* @dependencies
* - database
* @scenario
* - mock two transactions
* - insert tx with lower txId (with `approved` status)
* - run test (call `insertTx`)
* - check database
* @expected
* - tx should NOT be inserted into db
*/
it('should NOT insert tx when txId is higher than existing approved tx', async () => {
// mock two transactions
const chain = 'chain';
const txs = [
TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
),
TxTestData.mockPaymentTransaction(
TransactionType.coldStorage,
chain,
''
),
];
let highTx: PaymentTransaction;
let lowTx: PaymentTransaction;
if (txs[0].txId < txs[1].txId) {
highTx = txs[1];
lowTx = txs[0];
} else {
highTx = txs[0];
lowTx = txs[1];
}

// insert tx with lower txId (with `approved` status)
await DatabaseHandlerMock.insertTxRecord(
lowTx,
TransactionStatus.approved
);

// run test
await DatabaseHandler.insertTx(highTx, requiredSign);

// tx should NOT be inserted into db
const dbTxs = (await DatabaseHandlerMock.allTxRecords()).map(
(tx) => tx.txId
);
expect(dbTxs).toEqual([lowTx.txId]);
});

/**
* @target DatabaseHandler.insertColdStorageTx should update failedInSign when
* tx is already in database
Expand Down
14 changes: 14 additions & 0 deletions tests/handlers/ChainHandler.mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,20 @@ class ChainHandlerMock {
return this.getMockedChain(chainName)[name];
};

/**
* mocks a function for mocked chain and return the mock object
* @param chainName mocked chain name
* @param name function name
*/
static mockAndGetChainFunction = (
chainName: string,
name: string
): Mock<any> => {
const chain = this.getMockedChain(chainName);
chain[name] = vi.fn();
return chain[name];
};

/**
* returns a mocked function object
* @param name function name
Expand Down
Loading

0 comments on commit f8ab88b

Please sign in to comment.