CipherLabs
/
besu
mirror of https://github.com/hyperledger/besu
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix transaction tracking by sender (#93)

Browse Source 
Signed-off-by: Meredith Baxter <meredith.baxter@consensys.net>
pull/95/head
mbaxter 5 years ago committed by GitHub
parent 705298228b
commit 49e803c8dc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 92 additions and 12 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 39
      ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java
  2. 65
      ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java

39
ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactions.java
Unescape View File

@ -152,14 +152,7 @@ public class PendingTransactions {
final TransactionInfo removedTransactionInfo = pendingTransactions.remove(transaction.hash());
if (removedTransactionInfo != null) {
prioritizedTransactions.remove(removedTransactionInfo);
Optional.ofNullable(transactionsBySender.get(transaction.getSender()))
.ifPresent(
transactionsForSender -> {
transactionsForSender.remove(transaction.getNonce());
if (transactionsForSender.isEmpty()) {
transactionsBySender.remove(transaction.getSender());
}
});
removeTransactionTrackedBySenderAndNonce(transaction);
incrementTransactionRemovedCounter(
removedTransactionInfo.isReceivedFromLocalSource(), addedToBlock);
}
@ -240,20 +233,42 @@ public class PendingTransactions {
}
private boolean addTransactionForSenderAndNonce(final TransactionInfo transactionInfo) {
final Map<Long, TransactionInfo> transactionsForSender =
transactionsBySender.computeIfAbsent(transactionInfo.getSender(), key -> new TreeMap<>());
final TransactionInfo existingTransaction =
transactionsForSender.get(transactionInfo.getNonce());
getTrackedTransactionBySenderAndNonce(transactionInfo);
if (existingTransaction != null) {
if (!shouldReplace(existingTransaction, transactionInfo)) {
return false;
}
removeTransaction(existingTransaction.getTransaction());
}
transactionsForSender.put(transactionInfo.getNonce(), transactionInfo);
trackTransactionBySenderAndNonce(transactionInfo);
return true;
}
private void trackTransactionBySenderAndNonce(final TransactionInfo transactionInfo) {
final Map<Long, TransactionInfo> transactionsForSender =
transactionsBySender.computeIfAbsent(transactionInfo.getSender(), key -> new TreeMap<>());
transactionsForSender.put(transactionInfo.getNonce(), transactionInfo);
}
private void removeTransactionTrackedBySenderAndNonce(final Transaction transaction) {
Optional.ofNullable(transactionsBySender.get(transaction.getSender()))
.ifPresent(
transactionsForSender -> {
transactionsForSender.remove(transaction.getNonce());
if (transactionsForSender.isEmpty()) {
transactionsBySender.remove(transaction.getSender());
}
});
}
private TransactionInfo getTrackedTransactionBySenderAndNonce(
final TransactionInfo transactionInfo) {
final Map<Long, TransactionInfo> transactionsForSender =
transactionsBySender.computeIfAbsent(transactionInfo.getSender(), key -> new TreeMap<>());
return transactionsForSender.get(transactionInfo.getNonce());
}
private boolean shouldReplace(
final TransactionInfo existingTransaction, final TransactionInfo newTransaction) {
return newTransaction

65
ethereum/eth/src/test/java/org/hyperledger/besu/ethereum/eth/transactions/PendingTransactionsTest.java
Unescape View File

@ -325,6 +325,71 @@ public class PendingTransactionsTest {
assertThat(metricsSystem.getCounterValue(REMOVED_COUNTER, REMOTE, DROPPED)).isEqualTo(1);
}
@Test
public void shouldReplaceTransactionWithSameSenderAndNonce_multipleReplacements() {
final int replacedTxCount = 5;
final List<Transaction> replacedTransactions = new ArrayList<>();
for (int i = 0; i < replacedTxCount; i++) {
final Transaction duplicateTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, i + 1);
replacedTransactions.add(duplicateTx);
transactions.addRemoteTransaction(duplicateTx);
}
final Transaction finalReplacingTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, 100);
final Transaction independentTx = transactionWithNonceSenderAndGasPrice(2, KEYS1, 1);
assertThat(transactions.addRemoteTransaction(independentTx)).isTrue();
assertThat(transactions.addRemoteTransaction(finalReplacingTx)).isTrue();
// All tx's except the last duplicate should be removed
replacedTransactions.forEach(this::assertTransactionNotPending);
assertTransactionPending(finalReplacingTx);
// Tx with distinct nonce should be maintained
assertTransactionPending(independentTx);
assertThat(transactions.size()).isEqualTo(2);
assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, REMOTE)).isEqualTo(replacedTxCount + 2);
assertThat(metricsSystem.getCounterValue(REMOVED_COUNTER, REMOTE, DROPPED))
.isEqualTo(replacedTxCount);
}
@Test
public void
shouldReplaceTransactionWithSameSenderAndNonce_multipleReplacementsAddedLocallyAndRemotely() {
final int replacedTxCount = 11;
final List<Transaction> replacedTransactions = new ArrayList<>();
int remoteDuplicateCount = 0;
for (int i = 0; i < replacedTxCount; i++) {
final Transaction duplicateTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, i + 1);
replacedTransactions.add(duplicateTx);
if (i % 2 == 0) {
transactions.addRemoteTransaction(duplicateTx);
remoteDuplicateCount++;
} else {
transactions.addLocalTransaction(duplicateTx);
}
}
final Transaction finalReplacingTx = transactionWithNonceSenderAndGasPrice(1, KEYS1, 100);
final Transaction independentTx = transactionWithNonceSenderAndGasPrice(2, KEYS1, 1);
assertThat(transactions.addLocalTransaction(finalReplacingTx)).isTrue();
assertThat(transactions.addRemoteTransaction(independentTx)).isTrue();
// All tx's except the last duplicate should be removed
replacedTransactions.forEach(this::assertTransactionNotPending);
assertTransactionPending(finalReplacingTx);
// Tx with distinct nonce should be maintained
assertTransactionPending(independentTx);
final int localDuplicateCount = replacedTxCount - remoteDuplicateCount;
assertThat(transactions.size()).isEqualTo(2);
assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, REMOTE))
.isEqualTo(remoteDuplicateCount + 1);
assertThat(metricsSystem.getCounterValue(ADDED_COUNTER, LOCAL))
.isEqualTo(localDuplicateCount + 1);
assertThat(metricsSystem.getCounterValue(REMOVED_COUNTER, REMOTE, DROPPED))
.isEqualTo(remoteDuplicateCount);
assertThat(metricsSystem.getCounterValue(REMOVED_COUNTER, LOCAL, DROPPED))
.isEqualTo(localDuplicateCount);
}
@Test
public void shouldReplaceOnlyTransactionFromSenderWhenItHasTheSameNonce() {
final Transaction transaction1 = transactionWithNonceSenderAndGasPrice(1, KEYS1, 1);

Write Preview
Loading…
Cancel
Save