reject remote transactions when not in sync

PegaSysEng · Apr 10, 2019 · 886a07f · 886a07f
1 parent b519ee1
commit 886a07f
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 8 deletions.
diff --git a/ethereum/eth/build.gradle b/ethereum/eth/build.gradle
@@ -55,4 +55,7 @@ dependencies {
   testImplementation 'org.mockito:mockito-core'
 
   jmhImplementation project(path: ':ethereum:core', configuration: 'testSupportArtifacts')
+
+  testImplementation project(path: ':config', configuration: 'testSupportArtifacts')
+  integrationTestImplementation project(path: ':config', configuration: 'testSupportArtifacts')
 }
diff --git a/...um/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java b/...um/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java
@@ -28,6 +28,7 @@
 import tech.pegasys.pantheon.ethereum.core.PendingTransactionListener;
 import tech.pegasys.pantheon.ethereum.core.PendingTransactions;
 import tech.pegasys.pantheon.ethereum.core.Transaction;
+import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator;
 import tech.pegasys.pantheon.ethereum.mainnet.TransactionValidator.TransactionInvalidReason;
@@ -56,17 +57,20 @@ public class TransactionPool implements BlockAddedObserver {
   private final ProtocolSchedule<?> protocolSchedule;
   private final ProtocolContext<?> protocolContext;
   private final TransactionBatchAddedListener transactionBatchAddedListener;
+  private final SyncState syncState;
   private Optional<AccountFilter> accountFilter = Optional.empty();
 
   public TransactionPool(
       final PendingTransactions pendingTransactions,
       final ProtocolSchedule<?> protocolSchedule,
       final ProtocolContext<?> protocolContext,
-      final TransactionBatchAddedListener transactionBatchAddedListener) {
+      final TransactionBatchAddedListener transactionBatchAddedListener,
+      final SyncState syncState) {
     this.pendingTransactions = pendingTransactions;
     this.protocolSchedule = protocolSchedule;
     this.protocolContext = protocolContext;
     this.transactionBatchAddedListener = transactionBatchAddedListener;
+    this.syncState = syncState;
   }
 
   public ValidationResult<TransactionInvalidReason> addLocalTransaction(
@@ -87,6 +91,9 @@ public ValidationResult<TransactionInvalidReason> addLocalTransaction(
   public void addRemoteTransactions(final Collection<Transaction> transactions) {
     final Set<Transaction> addedTransactions = new HashSet<>();
     for (final Transaction transaction : sortByNonce(transactions)) {
+      if (!syncState.isInSync()) {
+        return;
+      }
       final ValidationResult<TransactionInvalidReason> validationResult =
           validateTransaction(transaction);
       if (validationResult.isValid()) {

diff --git a/...src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolFactory.java b/...src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolFactory.java
@@ -13,9 +13,11 @@
 package tech.pegasys.pantheon.ethereum.eth.transactions;
 
 import tech.pegasys.pantheon.ethereum.ProtocolContext;
+import tech.pegasys.pantheon.ethereum.chain.MutableBlockchain;
 import tech.pegasys.pantheon.ethereum.core.PendingTransactions;
 import tech.pegasys.pantheon.ethereum.eth.manager.EthContext;
 import tech.pegasys.pantheon.ethereum.eth.messages.EthPV62;
+import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.metrics.MetricsSystem;
 
@@ -36,13 +38,16 @@ public static TransactionPool createTransactionPool(
     final PeerTransactionTracker transactionTracker = new PeerTransactionTracker();
     final TransactionsMessageSender transactionsMessageSender =
         new TransactionsMessageSender(transactionTracker);
+    final MutableBlockchain blockchain = protocolContext.getBlockchain();
+    final SyncState syncState = new SyncState(blockchain, ethContext.getEthPeers());
 
     final TransactionPool transactionPool =
         new TransactionPool(
             pendingTransactions,
             protocolSchedule,
             protocolContext,
-            new TransactionSender(transactionTracker, transactionsMessageSender, ethContext));
+            new TransactionSender(transactionTracker, transactionsMessageSender, ethContext),
+            syncState);
 
     final TransactionsMessageHandler transactionsMessageHandler =
         new TransactionsMessageHandler(

diff --git a/...th/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java b/...th/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java
@@ -50,6 +50,7 @@
 import tech.pegasys.pantheon.ethereum.core.TransactionReceipt;
 import tech.pegasys.pantheon.ethereum.core.TransactionTestFixture;
 import tech.pegasys.pantheon.ethereum.core.Wei;
+import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
 import tech.pegasys.pantheon.ethereum.eth.transactions.TransactionPool.TransactionBatchAddedListener;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule;
 import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSpec;
@@ -87,21 +88,25 @@ public class TransactionPoolTest {
       new PendingTransactions(MAX_TRANSACTIONS, TestClock.fixed(), metricsSystem);
   private final Transaction transaction1 = createTransaction(1);
   private final Transaction transaction2 = createTransaction(2);
+  private final ExecutionContextTestFixture executionContext = ExecutionContextTestFixture.create();
+  private final ProtocolContext<Void> protocolContext = executionContext.getProtocolContext();
   private TransactionPool transactionPool;
+  private SyncState syncState;
   private long genesisBlockGasLimit;
   private final AccountFilter accountFilter = mock(AccountFilter.class);
 
   @Before
   public void setUp() {
-    final ExecutionContextTestFixture executionContext = ExecutionContextTestFixture.create();
     blockchain = executionContext.getBlockchain();
-    final ProtocolContext<Void> protocolContext = executionContext.getProtocolContext();
     when(protocolSchedule.getByBlockNumber(anyLong())).thenReturn(protocolSpec);
     when(protocolSpec.getTransactionValidator()).thenReturn(transactionValidator);
     genesisBlockGasLimit = executionContext.getGenesis().getHeader().getGasLimit();
+    syncState = mock(SyncState.class);
+    when(syncState.isInSync()).thenReturn(true);
 
     transactionPool =
-        new TransactionPool(transactions, protocolSchedule, protocolContext, batchAddedListener);
+        new TransactionPool(
+            transactions, protocolSchedule, protocolContext, batchAddedListener, syncState);
     blockchain.observeBlockAdded(transactionPool);
   }
 
@@ -165,7 +170,7 @@ public void shouldRemovePendingTransactionsFromAllBlocksOnAForkWhenItBecomesTheC
   }
 
   @Test
-  public void shouldReaddTransactionsFromThePreviousCanonicalHeadWhenAReorgOccurs() {
+  public void shouldReadTransactionsFromThePreviousCanonicalHeadWhenAReorgOccurs() {
     givenTransactionIsValid(transaction1);
     givenTransactionIsValid(transaction2);
     transactions.addRemoteTransaction(transaction1);
@@ -191,7 +196,7 @@ public void shouldReaddTransactionsFromThePreviousCanonicalHeadWhenAReorgOccurs(
   }
 
   @Test
-  public void shouldNotReaddTransactionsThatAreInBothForksWhenReorgHappens() {
+  public void shouldNotReadTransactionsThatAreInBothForksWhenReorgHappens() {
     givenTransactionIsValid(transaction1);
     givenTransactionIsValid(transaction2);
     transactions.addRemoteTransaction(transaction1);
@@ -410,6 +415,69 @@ public void shouldAllowTransactionWhenAccountWhitelistControllerIsNotPresent() {
     assertTransactionPending(transaction1);
   }
 
+  @Test
+  public void shouldRejectRemoteTransactionsWhenNotInSync() {
+    SyncState syncState = mock(SyncState.class);
+    when(syncState.isInSync()).thenReturn(false);
+    TransactionPool transactionPool =
+        new TransactionPool(
+            transactions, protocolSchedule, protocolContext, batchAddedListener, syncState);
+
+    final TransactionTestFixture builder = new TransactionTestFixture();
+    final Transaction transaction1 = builder.nonce(1).createTransaction(KEY_PAIR1);
+    final Transaction transaction2 = builder.nonce(2).createTransaction(KEY_PAIR1);
+    final Transaction transaction3 = builder.nonce(3).createTransaction(KEY_PAIR1);
+
+    when(transactionValidator.validate(any(Transaction.class))).thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            eq(transaction1), nullable(Account.class), eq(true)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            eq(transaction2), nullable(Account.class), eq(true)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            eq(transaction3), nullable(Account.class), eq(true)))
+        .thenReturn(valid());
+
+    transactionPool.addRemoteTransactions(asList(transaction3, transaction1, transaction2));
+
+    assertTransactionNotPending(transaction1);
+    assertTransactionNotPending(transaction2);
+    assertTransactionNotPending(transaction3);
+    verifyZeroInteractions(batchAddedListener);
+  }
+
+  @Test
+  public void shouldAllowRemoteTransactionsWhenInSync() {
+    SyncState syncState = mock(SyncState.class);
+    when(syncState.isInSync()).thenReturn(true);
+    TransactionPool transactionPool =
+        new TransactionPool(
+            transactions, protocolSchedule, protocolContext, batchAddedListener, syncState);
+
+    final TransactionTestFixture builder = new TransactionTestFixture();
+    final Transaction transaction1 = builder.nonce(1).createTransaction(KEY_PAIR1);
+    final Transaction transaction2 = builder.nonce(2).createTransaction(KEY_PAIR1);
+    final Transaction transaction3 = builder.nonce(3).createTransaction(KEY_PAIR1);
+
+    when(transactionValidator.validate(any(Transaction.class))).thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            eq(transaction1), nullable(Account.class), eq(true)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            eq(transaction2), nullable(Account.class), eq(true)))
+        .thenReturn(valid());
+    when(transactionValidator.validateForSender(
+            eq(transaction3), nullable(Account.class), eq(true)))
+        .thenReturn(valid());
+
+    transactionPool.addRemoteTransactions(asList(transaction3, transaction1, transaction2));
+
+    assertTransactionPending(transaction1);
+    assertTransactionPending(transaction2);
+    assertTransactionPending(transaction3);
+  }
+
   private void assertTransactionPending(final Transaction t) {
     assertThat(transactions.getTransactionByHash(t.hash())).contains(t);
   }

diff --git a/...va/tech/pegasys/pantheon/ethereum/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java b/...va/tech/pegasys/pantheon/ethereum/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java
@@ -16,6 +16,8 @@
 import static java.util.Collections.emptyList;
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import tech.pegasys.pantheon.crypto.SECP256K1.KeyPair;
 import tech.pegasys.pantheon.ethereum.ProtocolContext;
@@ -30,6 +32,7 @@
 import tech.pegasys.pantheon.ethereum.core.Transaction;
 import tech.pegasys.pantheon.ethereum.core.TransactionReceipt;
 import tech.pegasys.pantheon.ethereum.core.Wei;
+import tech.pegasys.pantheon.ethereum.eth.sync.state.SyncState;
 import tech.pegasys.pantheon.ethereum.eth.transactions.TransactionPool;
 import tech.pegasys.pantheon.ethereum.eth.transactions.TransactionPool.TransactionBatchAddedListener;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.JsonRpcRequest;
@@ -76,18 +79,23 @@ public class EthGetFilterChangesIntegrationTest {
   private final JsonRpcParameter parameters = new JsonRpcParameter();
   private FilterManager filterManager;
   private EthGetFilterChanges method;
+  private final SyncState syncState = mock(SyncState.class);
 
   @Before
   public void setUp() {
     final ExecutionContextTestFixture executionContext = ExecutionContextTestFixture.create();
     blockchain = executionContext.getBlockchain();
     final ProtocolContext<Void> protocolContext = executionContext.getProtocolContext();
+
+    when(syncState.isInSync()).thenReturn(true);
+
     transactionPool =
         new TransactionPool(
             transactions,
             executionContext.getProtocolSchedule(),
             protocolContext,
-            batchAddedListener);
+            batchAddedListener,
+            syncState);
     final BlockchainQueries blockchainQueries =
         new BlockchainQueries(blockchain, protocolContext.getWorldStateArchive());
     filterManager =