From 886a07f1ae4798bb407250b054f930ac2f42a78d Mon Sep 17 00:00:00 2001 From: "S. Matthew English" Date: Wed, 10 Apr 2019 03:20:47 -0400 Subject: [PATCH] reject remote transactions when not in sync --- ethereum/eth/build.gradle | 3 + .../eth/transactions/TransactionPool.java | 9 ++- .../transactions/TransactionPoolFactory.java | 7 +- .../eth/transactions/TransactionPoolTest.java | 78 +++++++++++++++++-- .../EthGetFilterChangesIntegrationTest.java | 10 ++- 5 files changed, 99 insertions(+), 8 deletions(-) diff --git a/ethereum/eth/build.gradle b/ethereum/eth/build.gradle index 0edc1cbb75..1eff9e8b90 100644 --- 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/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java index 5c1089058a..aa59e0fe08 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPool.java +++ b/ethereum/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 = 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 addLocalTransaction( @@ -87,6 +91,9 @@ public ValidationResult addLocalTransaction( public void addRemoteTransactions(final Collection transactions) { final Set addedTransactions = new HashSet<>(); for (final Transaction transaction : sortByNonce(transactions)) { + if (!syncState.isInSync()) { + return; + } final ValidationResult validationResult = validateTransaction(transaction); if (validationResult.isValid()) { diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolFactory.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolFactory.java index 8403713763..afa412a474 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolFactory.java +++ b/ethereum/eth/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/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java index 88e95c3830..9281e05094 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/transactions/TransactionPoolTest.java +++ b/ethereum/eth/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 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 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/ethereum/jsonrpc/src/integration-test/java/tech/pegasys/pantheon/ethereum/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java b/ethereum/jsonrpc/src/integration-test/java/tech/pegasys/pantheon/ethereum/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java index 80406bac8b..8239f66c23 100644 --- a/ethereum/jsonrpc/src/integration-test/java/tech/pegasys/pantheon/ethereum/jsonrpc/methods/EthGetFilterChangesIntegrationTest.java +++ b/ethereum/jsonrpc/src/integration-test/java/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 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 =