Updated transaction validation to check onchain permissions at the tr…

…ansaction pool level (hyperledger#2595) * Updated transaction validation to check onchain permissions at the transaction pool level. Signed-off-by: Mark Terry <mark.terry@consensys.net>
shemnon · Aug 3, 2021 · 5645c2d · 5645c2d
1 parent dfd82be
commit 5645c2d
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,10 +7,10 @@
 - `eth_getWork`, `eth_submitWork` support over the Stratum port [#2581](https://github.com/hyperledger/besu/pull/2581)
 - Stratum metrics [#2583](https://github.com/hyperledger/besu/pull/2583)
 - Support for mining ommers [#2576](https://github.com/hyperledger/besu/pull/2576)
-
+- Updated onchain permissioning to validate permissions on transaction submission [\#2595](https://github.com/hyperledger/besu/pull/2595)
 
 ### Bug Fixes
-- consider effective price and effective priority fee in transaction replacement rules [\#2529](https://github.com/hyperledger/besu/issues/2529)
+- Consider effective price and effective priority fee in transaction replacement rules [\#2529](https://github.com/hyperledger/besu/issues/2529)
 
 ### Early Access Features
 

diff --git a/...esu/tests/acceptance/permissioning/AccountLocalAndOnChainPermissioningAcceptanceTest.java b/...esu/tests/acceptance/permissioning/AccountLocalAndOnChainPermissioningAcceptanceTest.java
@@ -99,9 +99,8 @@ public void testAccountCannotSendTxWhenNotOnOnChainAllowList() {
     // verify senderC is forbidden because it is not on OnChain allowlist
     node.verify(accountIsForbidden(senderC));
 
-    // sender C should not be able to send Tx as well
-    node.execute(accountTransactions.createTransfer(senderC, receiverAccount, 1));
-    node.verify(receiverAccount.balanceDoesNotChange(0));
+    // sender C should not be able to send Tx
+    verifyTransferForbidden(node, senderC, accounts.getSecondaryBenefactor());
 
     // final check, other account should be able to send tx
     node.execute(
@@ -111,8 +110,8 @@ public void testAccountCannotSendTxWhenNotOnOnChainAllowList() {
 
   private void verifyTransferForbidden(
       final Node node, final Account sender, final Account beneficiary) {
-    BigInteger nonce = node.execute(ethTransactions.getTransactionCount(sender.getAddress()));
-    TransferTransaction transfer =
+    final BigInteger nonce = node.execute(ethTransactions.getTransactionCount(sender.getAddress()));
+    final TransferTransaction transfer =
         accountTransactions.createTransfer(sender, beneficiary, 1, nonce);
     node.verify(
         eth.sendRawTransactionExceptional(

diff --git a/.../besu/tests/acceptance/permissioning/AccountSmartContractPermissioningAcceptanceTest.java b/.../besu/tests/acceptance/permissioning/AccountSmartContractPermissioningAcceptanceTest.java
@@ -16,7 +16,9 @@
 
 import org.hyperledger.besu.tests.acceptance.dsl.account.Account;
 import org.hyperledger.besu.tests.acceptance.dsl.node.Node;
+import org.hyperledger.besu.tests.acceptance.dsl.transaction.account.TransferTransaction;
 
+import java.math.BigInteger;
 import java.util.Collections;
 
 import org.junit.Before;
@@ -60,7 +62,16 @@ public void forbiddenAccountCannotTransferValue() {
     node.execute(forbidAccount(allowedSender));
     node.verify(accountIsForbidden(allowedSender));
 
-    node.execute(accountTransactions.createTransfer(allowedSender, otherAccount, 5));
-    node.verify(otherAccount.balanceDoesNotChange(0));
+    verifyTransferForbidden(allowedSender, otherAccount);
+  }
+
+  private void verifyTransferForbidden(final Account sender, final Account beneficiary) {
+    final BigInteger nonce = node.execute(ethTransactions.getTransactionCount(sender.getAddress()));
+    final TransferTransaction transfer =
+        accountTransactions.createTransfer(sender, beneficiary, 1, nonce);
+    node.verify(
+        eth.sendRawTransactionExceptional(
+            transfer.signedTransactionData(),
+            "Sender account not authorized to send transactions"));
   }
 }
diff --git a/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParams.java b/...core/src/main/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParams.java
@@ -24,7 +24,7 @@ public interface TransactionValidationParams {
       ImmutableTransactionValidationParams.of(false, false, false, true, false);
 
   TransactionValidationParams transactionPoolParams =
-      ImmutableTransactionValidationParams.of(true, false, true, false, true);
+      ImmutableTransactionValidationParams.of(true, false, true, true, true);
 
   TransactionValidationParams miningParams =
       ImmutableTransactionValidationParams.of(false, false, false, true, true);

diff --git a/.../src/test/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParamsTest.java b/.../src/test/java/org/hyperledger/besu/ethereum/mainnet/TransactionValidationParamsTest.java
@@ -88,7 +88,7 @@ public void processingBlock() {
   public void transactionPool() {
     final TransactionValidationParams params = TransactionValidationParams.transactionPool();
     assertThat(params.isAllowFutureNonce()).isTrue();
-    assertThat(params.checkOnchainPermissions()).isFalse();
+    assertThat(params.checkOnchainPermissions()).isTrue();
     assertThat(params.checkLocalPermissions()).isTrue();
   }