# 2023 02 27 NEM Stall Post Mortem

## Observation

After the production of block 4129631, block production on the NEM network slowed to a crawl. This situation was instigated by an unexpected transaction entering the Unconfirmed Transactions cache. When this transaction expired, block production went back to normal.

We have identified the makeup of the unexpected transaction and how it entered the Unconfirmed Transactions cache.

A multisig account configured with 1-of-2 multisig sent two conflicting transactaions. Assume that the two cosigners are named A and B and the multisig account is named M.

## Root Cause Analysis

The following sequence of events occured:

1. M sends a multisig transaction MT with an inner transaction - X - signed by A

1. The multisig transaction gets committed to the chain and TransactionHashesObserver (running as part of BlockTransactionFactory) adds both H(MT), H(X) to HashTransactionsCache

    public void notify(final TransactionHashesNotification notification, final BlockNotificationContext context) {
	if (NotificationTrigger.Execute == context.getTrigger()) {
		this.transactionHashCache.putAll(notification.getPairs());
	} 
    ...

Importantly, when the `TransactionsHashesNotification` is created, `streamDefault` is used, which expands to both outer and inner transactions:

	protected void notifyTransactionHashes() {
		final List<HashMetaDataPair> pairs = BlockExtensions.streamDefault(this.block)
				.map(t -> new HashMetaDataPair(HashUtils.calculateHash(t), new HashMetaData(this.block.getHeight(), t.getTimeStamp())))
				.collect(Collectors.toList());
		this.observer.notify(new TransactionHashesNotification(pairs));
	}

1. M sends a multisig transaction MT' with the **SAME** inner transaction X signed by B

1. `BatchUniqueHashTransactionValidator` checks that H(MT') is not in HashTransactionsCache, so it gets added to UnconfirmedTransactionsCache ✅

	public ValidationResult validate(final List<TransactionsContextPair> groupedTransactions) {
            ...
		final List<Hash> hashes = groupedTransactions.stream().flatMap(pair -> pair.getTransactions().stream())
				.map(HashUtils::calculateHash).collect(Collectors.toList());

		return this.validate(hashes);
	}

Importantly, `pair.getTransactions()` returns ` Collection<Transaction>`, which only contains outer transactions. As a result, H(MT') != H(MT) is checked but H(X) is not checked.

1. MT' gets selected for a block by the harvester. `BlockUniqueHashTransactionValidator` has the same logic as `BatchUniqueHashTransactionValidator`, so only the hash of the outer MT' is checked. As a result, the transaction is included in the harvested block ✅

	public ValidationResult validate(final Block 
            ...
		final List<Hash> hashes = block.getTransactions().stream().map(HashUtils::calculateHash).collect(Collectors.toList());
		return this.transactionHashCache.anyHashExists(hashes) ? ValidationResult.NEUTRAL : ValidationResult.SUCCESS;
	}

1. When the newly harvested block - containing MT' - is processed, `TransactionHashesObserver` attempts to add both H(MT') and H(X) to `HashTransactionsCache` during part of the block processing that makes state changes. This fails because H(X) is already present in the `UnconfirmedTransactionsCache`, so the harvested block gets rejected 🛑

1. Go to step 5 until H(MT') expires out of the `UnconfirmedTransactionsCache`


## Fix

The fix is to update both of the unique hash transaction validators to check both outer and inner transactions hashes. This can be done simply by using `TransactionExtensions.streamDefault()`.

### BatchUniqueHashTransactionValidator

The addition of `TransactionExtensions.streamDefault` will prevent MT' (or similar) from getting added to the `UnconfirmedTransactionsCache.

public ValidationResult validate(final List<TransactionsContextPair> groupedTransactions) {
    if (groupedTransactions.isEmpty()) {
        return ValidationResult.SUCCESS;
    }

    final List<Hash> hashes = groupedTransactions.stream().flatMap(pair -> pair.getTransactions().stream())
            .flatMap(transaction -> TransactionExtensions.streamDefault(transaction)).map(HashUtils::calculateHash)
            .collect(Collectors.toList());
    return this.validate(hashes);
}

### BlockUniqueHashTransactionValidator

The addition of `TransactionExtensions.streamDefault` will prevent MT' (or similar) from getting added to a newly harvested block.

public ValidationResult validate(final Block block) {
    if (block.getTransactions().isEmpty()) {
        return ValidationResult.SUCCESS;
    }

    final List<Hash> hashes = block.getTransactions().stream().flatMap(transaction -> TransactionExtensions.streamDefault(transaction))
            .map(HashUtils::calculateHash).collect(Collectors.toList());
    return this.transactionHashCache.anyHashExists(hashes) ? ValidationResult.NEUTRAL : ValidationResult.SUCCESS;
}