From d3259890176759147b552e06548aa017f1f1153e Mon Sep 17 00:00:00 2001 From: Chris Mckay Date: Mon, 27 May 2019 06:38:43 +1000 Subject: [PATCH] [PIE-1580] Metrics for smart contract permissioning actions (#1492) Signed-off-by: Adrian Sutton --- ethereum/permissioning/build.gradle | 1 + .../NodePermissioningControllerFactory.java | 10 ++- ...eSmartContractPermissioningController.java | 36 +++++++- ...nSmartContractPermissioningController.java | 36 +++++++- .../SyncStatusNodePermissioningProvider.java | 40 ++++++++- ...rtContractPermissioningControllerTest.java | 90 ++++++++++++++++++- ...rtContractPermissioningControllerTest.java | 75 +++++++++++++++- ...odePermissioningControllerFactoryTest.java | 36 ++++++-- ...ncStatusNodePermissioningProviderTest.java | 60 ++++++++++++- .../pantheon/metrics/MetricCategory.java | 1 + .../tech/pegasys/pantheon/RunnerBuilder.java | 8 +- 11 files changed, 375 insertions(+), 18 deletions(-) diff --git a/ethereum/permissioning/build.gradle b/ethereum/permissioning/build.gradle index 97e402265c..54bfe7e19c 100644 --- a/ethereum/permissioning/build.gradle +++ b/ethereum/permissioning/build.gradle @@ -29,6 +29,7 @@ dependencies { implementation project(':util') implementation project(':ethereum:core') implementation project(':crypto') + implementation project(':metrics:core') implementation 'com.google.guava:guava' implementation 'net.consensys.cava:cava-toml' diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java index db48e7a7a0..823bd298e1 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodePermissioningControllerFactory.java @@ -17,6 +17,7 @@ import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningContro import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningProvider; import tech.pegasys.pantheon.ethereum.permissioning.node.provider.SyncStatusNodePermissioningProvider; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulator; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.enode.EnodeURL; @@ -32,7 +33,8 @@ public class NodePermissioningControllerFactory { final Synchronizer synchronizer, final Collection fixedNodes, final BytesValue localNodeId, - final TransactionSimulator transactionSimulator) { + final TransactionSimulator transactionSimulator, + final MetricsSystem metricsSystem) { Optional syncStatusProviderOptional; @@ -55,7 +57,8 @@ public class NodePermissioningControllerFactory { NodeSmartContractPermissioningController smartContractProvider = new NodeSmartContractPermissioningController( smartContractPermissioningConfiguration.getNodeSmartContractAddress(), - transactionSimulator); + transactionSimulator, + metricsSystem); providers.add(smartContractProvider); } @@ -63,7 +66,8 @@ public class NodePermissioningControllerFactory { syncStatusProviderOptional = Optional.empty(); } else { syncStatusProviderOptional = - Optional.of(new SyncStatusNodePermissioningProvider(synchronizer, fixedNodes)); + Optional.of( + new SyncStatusNodePermissioningProvider(synchronizer, fixedNodes, metricsSystem)); } } else { syncStatusProviderOptional = Optional.empty(); diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java index 78e34fa7de..34187d88b2 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningController.java @@ -20,6 +20,9 @@ import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningProvid import tech.pegasys.pantheon.ethereum.transaction.CallParameter; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulator; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulatorResult; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.bytes.BytesValues; import tech.pegasys.pantheon.util.enode.EnodeURL; @@ -40,6 +43,9 @@ public class NodeSmartContractPermissioningController implements NodePermissioni "connectionAllowed(bytes32,bytes32,bytes16,uint16,bytes32,bytes32,bytes16,uint16)"; // hashed function signature for connection allowed call private static final BytesValue FUNCTION_SIGNATURE_HASH = hashSignature(FUNCTION_SIGNATURE); + private final Counter checkCounter; + private final Counter checkCounterPermitted; + private final Counter checkCounterUnpermitted; // The first 4 bytes of the hash of the full textual signature of the function is used in // contract calls to determine the function being called @@ -60,11 +66,30 @@ public class NodeSmartContractPermissioningController implements NodePermissioni * * @param contractAddress The address at which the permissioning smart contract resides * @param transactionSimulator A transaction simulator with attached blockchain and world state + * @param metricsSystem The metrics provider that is to be reported to */ public NodeSmartContractPermissioningController( - final Address contractAddress, final TransactionSimulator transactionSimulator) { + final Address contractAddress, + final TransactionSimulator transactionSimulator, + final MetricsSystem metricsSystem) { this.contractAddress = contractAddress; this.transactionSimulator = transactionSimulator; + + this.checkCounter = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_smart_contract_check_count", + "Number of times the node smart contract permissioning provider has been checked"); + this.checkCounterPermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_smart_contract_check_count_permitted", + "Number of times the node smart contract permissioning provider has been checked and returned permitted"); + this.checkCounterUnpermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_smart_contract_check_count_unpermitted", + "Number of times the node smart contract permissioning provider has been checked and returned unpermitted"); } /** @@ -76,6 +101,7 @@ public class NodeSmartContractPermissioningController implements NodePermissioni */ @Override public boolean isPermitted(final EnodeURL sourceEnode, final EnodeURL destinationEnode) { + this.checkCounter.inc(); final BytesValue payload = createPayload(sourceEnode, destinationEnode); final CallParameter callParams = new CallParameter(null, contractAddress, -1, null, null, payload); @@ -101,7 +127,13 @@ public class NodeSmartContractPermissioningController implements NodePermissioni } } - return result.map(r -> checkTransactionResult(r.getOutput())).orElse(false); + if (result.map(r -> checkTransactionResult(r.getOutput())).orElse(false)) { + this.checkCounterPermitted.inc(); + return true; + } else { + this.checkCounterUnpermitted.inc(); + return false; + } } // Checks the returned bytes from the permissioning contract call to see if it's a value we diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningController.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningController.java index 0f4d83f3e6..9eea6695db 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningController.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningController.java @@ -22,6 +22,9 @@ import tech.pegasys.pantheon.ethereum.permissioning.account.TransactionPermissio import tech.pegasys.pantheon.ethereum.transaction.CallParameter; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulator; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulatorResult; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; import tech.pegasys.pantheon.util.bytes.BytesValues; @@ -41,6 +44,9 @@ public class TransactionSmartContractPermissioningController "transactionAllowed(address,address,uint256,uint256,uint256,bytes)"; // hashed function signature for connection allowed call private static final BytesValue FUNCTION_SIGNATURE_HASH = hashSignature(FUNCTION_SIGNATURE); + private final Counter checkCounterPermitted; + private final Counter checkCounter; + private final Counter checkCounterUnpermitted; // The first 4 bytes of the hash of the full textual signature of the function is used in // contract calls to determine the function being called @@ -61,11 +67,30 @@ public class TransactionSmartContractPermissioningController * * @param contractAddress The address at which the permissioning smart contract resides * @param transactionSimulator A transaction simulator with attached blockchain and world state + * @param metricsSystem The metrics provider that is to be reported to */ public TransactionSmartContractPermissioningController( - final Address contractAddress, final TransactionSimulator transactionSimulator) { + final Address contractAddress, + final TransactionSimulator transactionSimulator, + final MetricsSystem metricsSystem) { this.contractAddress = contractAddress; this.transactionSimulator = transactionSimulator; + + this.checkCounter = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "transaction_smart_contract_check_count", + "Number of times the transaction smart contract permissioning provider has been checked"); + this.checkCounterPermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "transaction_smart_contract_check_count_permitted", + "Number of times the transaction smart contract permissioning provider has been checked and returned permitted"); + this.checkCounterUnpermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "transaction_smart_contract_check_count_unpermitted", + "Number of times the transaction smart contract permissioning provider has been checked and returned unpermitted"); } /** @@ -76,6 +101,7 @@ public class TransactionSmartContractPermissioningController */ @Override public boolean isPermitted(final Transaction transaction) { + this.checkCounter.inc(); final BytesValue payload = createPayload(transaction); final CallParameter callParams = new CallParameter(null, contractAddress, -1, null, null, payload); @@ -103,7 +129,13 @@ public class TransactionSmartContractPermissioningController } } - return result.map(r -> checkTransactionResult(r.getOutput())).orElse(false); + if (result.map(r -> checkTransactionResult(r.getOutput())).orElse(false)) { + this.checkCounterPermitted.inc(); + return true; + } else { + this.checkCounterUnpermitted.inc(); + return false; + } } // Checks the returned bytes from the permissioning contract call to see if it's a value we diff --git a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java index 343bbdbee4..8019aa8392 100644 --- a/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java +++ b/ethereum/permissioning/src/main/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProvider.java @@ -17,6 +17,9 @@ import static com.google.common.base.Preconditions.checkNotNull; import tech.pegasys.pantheon.ethereum.core.SyncStatus; import tech.pegasys.pantheon.ethereum.core.Synchronizer; import tech.pegasys.pantheon.ethereum.permissioning.node.NodePermissioningProvider; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.util.Collection; @@ -27,16 +30,42 @@ public class SyncStatusNodePermissioningProvider implements NodePermissioningPro private final Synchronizer synchronizer; private final Collection fixedNodes = new HashSet<>(); + private final Counter checkCounter; + private final Counter checkCounterPermitted; + private final Counter checkCounterUnpermitted; private OptionalLong syncStatusObserverId; private boolean hasReachedSync = false; public SyncStatusNodePermissioningProvider( - final Synchronizer synchronizer, final Collection fixedNodes) { + final Synchronizer synchronizer, + final Collection fixedNodes, + final MetricsSystem metricsSystem) { checkNotNull(synchronizer); this.synchronizer = synchronizer; long id = this.synchronizer.observeSyncStatus(this::handleSyncStatusUpdate); this.syncStatusObserverId = OptionalLong.of(id); this.fixedNodes.addAll(fixedNodes); + + metricsSystem.createIntegerGauge( + MetricCategory.PERMISSIONING, + "sync_status_node_sync_reached", + "Whether the sync status permissioning provider has realised sync yet", + () -> hasReachedSync ? 1 : 0); + this.checkCounter = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "sync_status_node_check_count", + "Number of times the sync status permissioning provider has been checked"); + this.checkCounterPermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "sync_status_node_check_count_permitted", + "Number of times the sync status permissioning provider has been checked and returned permitted"); + this.checkCounterUnpermitted = + metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "sync_status_node_check_count_unpermitted", + "Number of times the sync status permissioning provider has been checked and returned unpermitted"); } private void handleSyncStatusUpdate(final SyncStatus syncStatus) { @@ -73,7 +102,14 @@ public class SyncStatusNodePermissioningProvider implements NodePermissioningPro if (hasReachedSync) { return true; } else { - return fixedNodes.contains(destinationEnode); + checkCounter.inc(); + if (fixedNodes.contains(destinationEnode)) { + checkCounterPermitted.inc(); + return true; + } else { + checkCounterUnpermitted.inc(); + return false; + } } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningControllerTest.java index 5c621a8b4a..2883c4022c 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/NodeSmartContractPermissioningControllerTest.java @@ -15,6 +15,9 @@ package tech.pegasys.pantheon.ethereum.permissioning; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive; @@ -26,14 +29,25 @@ import tech.pegasys.pantheon.ethereum.mainnet.MainnetProtocolSchedule; import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulator; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.io.IOException; import com.google.common.io.Resources; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +@RunWith(MockitoJUnitRunner.StrictStubs.class) public class NodeSmartContractPermissioningControllerTest { + @Mock private MetricsSystem metricsSystem; + @Mock private Counter checkCounter; + @Mock private Counter checkPermittedCounter; + @Mock private Counter checkUnpermittedCounter; private NodeSmartContractPermissioningController setupController( final String resourceName, final String contractAddressString) throws IOException { @@ -53,7 +67,49 @@ public class NodeSmartContractPermissioningControllerTest { new TransactionSimulator(blockchain, worldArchive, protocolSchedule); final Address contractAddress = Address.fromHexString(contractAddressString); - return new NodeSmartContractPermissioningController(contractAddress, ts); + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_smart_contract_check_count", + "Number of times the node smart contract permissioning provider has been checked")) + .thenReturn(checkCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_smart_contract_check_count_permitted", + "Number of times the node smart contract permissioning provider has been checked and returned permitted")) + .thenReturn(checkPermittedCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "node_smart_contract_check_count_unpermitted", + "Number of times the node smart contract permissioning provider has been checked and returned unpermitted")) + .thenReturn(checkUnpermittedCounter); + + return new NodeSmartContractPermissioningController(contractAddress, ts, metricsSystem); + } + + private void verifyCountersUntouched() { + verify(checkCounter, times(0)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + + private void verifyCountersPermitted() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(1)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + + private void verifyCountersUnpermitted() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(1)).inc(); + } + + private void verifyCountersFailedCheck() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); } @Test @@ -63,6 +119,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat( controller.isPermitted( EnodeURL.fromString( @@ -70,6 +128,8 @@ public class NodeSmartContractPermissioningControllerTest { EnodeURL.fromString( "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30304"))) .isTrue(); + + verifyCountersPermitted(); } @Test @@ -79,6 +139,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat( controller.isPermitted( EnodeURL.fromString( @@ -86,6 +148,8 @@ public class NodeSmartContractPermissioningControllerTest { EnodeURL.fromString( "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30305"))) .isFalse(); + + verifyCountersUnpermitted(); } @Test @@ -95,6 +159,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat( controller.isPermitted( EnodeURL.fromString( @@ -102,6 +168,8 @@ public class NodeSmartContractPermissioningControllerTest { EnodeURL.fromString( "enode://6f8a80d14311c39f35f516fa664deaaaa13e85b2f7493f37f6144d86991ec012937307647bd3b9a82abe2974e1407241d54947bbb39763a4cac9f77166ad92a0@192.168.0.1:30304"))) .isFalse(); + + verifyCountersUnpermitted(); } @Test @@ -111,6 +179,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat( controller.isPermitted( EnodeURL.fromString( @@ -118,6 +188,8 @@ public class NodeSmartContractPermissioningControllerTest { EnodeURL.fromString( "enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab62@[1:2:3:4:5:6:7:8]:30304"))) .isTrue(); + + verifyCountersPermitted(); } @Test @@ -127,6 +199,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat( controller.isPermitted( EnodeURL.fromString( @@ -134,6 +208,8 @@ public class NodeSmartContractPermissioningControllerTest { EnodeURL.fromString( "enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab62@[1:2:3:4:5:6:7:8]:30304"))) .isFalse(); + + verifyCountersUnpermitted(); } @Test @@ -143,6 +219,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat( controller.isPermitted( EnodeURL.fromString( @@ -150,6 +228,8 @@ public class NodeSmartContractPermissioningControllerTest { EnodeURL.fromString( "enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab63@[1:2:3:4:5:6:7:8]:30304"))) .isFalse(); + + verifyCountersUnpermitted(); } @Test @@ -159,6 +239,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/noSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThatThrownBy( () -> controller.isPermitted( @@ -168,6 +250,8 @@ public class NodeSmartContractPermissioningControllerTest { "enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab63@[1:2:3:4:5:6:7:8]:30304"))) .isInstanceOf(IllegalStateException.class) .hasMessage("Permissioning contract does not exist"); + + verifyCountersFailedCheck(); } @Test @@ -177,6 +261,8 @@ public class NodeSmartContractPermissioningControllerTest { "/NodeSmartContractPermissioningControllerTest/corruptSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThatThrownBy( () -> controller.isPermitted( @@ -186,5 +272,7 @@ public class NodeSmartContractPermissioningControllerTest { "enode://1234000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000ab63@[1:2:3:4:5:6:7:8]:30304"))) .isInstanceOf(IllegalStateException.class) .hasMessage("Permissioning transaction failed when processing"); + + verifyCountersFailedCheck(); } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningControllerTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningControllerTest.java index ed5300fe5c..7d364d33d4 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningControllerTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/TransactionSmartContractPermissioningControllerTest.java @@ -15,6 +15,9 @@ package tech.pegasys.pantheon.ethereum.permissioning; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryBlockchain; import static tech.pegasys.pantheon.ethereum.core.InMemoryStorageProvider.createInMemoryWorldStateArchive; @@ -28,14 +31,26 @@ import tech.pegasys.pantheon.ethereum.mainnet.MainnetProtocolSchedule; import tech.pegasys.pantheon.ethereum.mainnet.ProtocolSchedule; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulator; import tech.pegasys.pantheon.ethereum.worldstate.WorldStateArchive; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.bytes.BytesValue; import java.io.IOException; import com.google.common.io.Resources; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; +@RunWith(MockitoJUnitRunner.StrictStubs.class) public class TransactionSmartContractPermissioningControllerTest { + @Mock private MetricsSystem metricsSystem; + @Mock private Counter checkCounter; + @Mock private Counter checkPermittedCounter; + @Mock private Counter checkUnpermittedCounter; + private TransactionSmartContractPermissioningController setupController( final String resourceName, final String contractAddressString) throws IOException { final ProtocolSchedule protocolSchedule = MainnetProtocolSchedule.create(); @@ -54,7 +69,25 @@ public class TransactionSmartContractPermissioningControllerTest { new TransactionSimulator(blockchain, worldArchive, protocolSchedule); final Address contractAddress = Address.fromHexString(contractAddressString); - return new TransactionSmartContractPermissioningController(contractAddress, ts); + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "transaction_smart_contract_check_count", + "Number of times the transaction smart contract permissioning provider has been checked")) + .thenReturn(checkCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "transaction_smart_contract_check_count_permitted", + "Number of times the transaction smart contract permissioning provider has been checked and returned permitted")) + .thenReturn(checkPermittedCounter); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "transaction_smart_contract_check_count_unpermitted", + "Number of times the transaction smart contract permissioning provider has been checked and returned unpermitted")) + .thenReturn(checkUnpermittedCounter); + + return new TransactionSmartContractPermissioningController(contractAddress, ts, metricsSystem); } private Transaction transactionForAccount(final Address address) { @@ -67,6 +100,30 @@ public class TransactionSmartContractPermissioningControllerTest { .build(); } + private void verifyCountersUntouched() { + verify(checkCounter, times(0)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + + private void verifyCountersPermitted() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(1)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + + private void verifyCountersUnpermitted() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(1)).inc(); + } + + private void verifyCountersFailedCheck() { + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); + } + @Test public void testAccountIncluded() throws IOException { final TransactionSmartContractPermissioningController controller = @@ -74,8 +131,12 @@ public class TransactionSmartContractPermissioningControllerTest { "/TransactionSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat(controller.isPermitted(transactionForAccount(Address.fromHexString("0x1")))) .isTrue(); + + verifyCountersPermitted(); } @Test @@ -85,8 +146,12 @@ public class TransactionSmartContractPermissioningControllerTest { "/TransactionSmartContractPermissioningControllerTest/preseededSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThat(controller.isPermitted(transactionForAccount(Address.fromHexString("0x2")))) .isFalse(); + + verifyCountersUnpermitted(); } @Test @@ -96,10 +161,14 @@ public class TransactionSmartContractPermissioningControllerTest { "/TransactionSmartContractPermissioningControllerTest/noSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThatThrownBy( () -> controller.isPermitted(transactionForAccount(Address.fromHexString("0x1")))) .isInstanceOf(IllegalStateException.class) .hasMessage("Transaction permissioning contract does not exist"); + + verifyCountersFailedCheck(); } @Test @@ -109,9 +178,13 @@ public class TransactionSmartContractPermissioningControllerTest { "/TransactionSmartContractPermissioningControllerTest/corruptSmartPermissioning.json", "0x0000000000000000000000000000000000001234"); + verifyCountersUntouched(); + assertThatThrownBy( () -> controller.isPermitted(transactionForAccount(Address.fromHexString("0x1")))) .isInstanceOf(IllegalStateException.class) .hasMessage("Transaction permissioning transaction failed when processing"); + + verifyCountersFailedCheck(); } } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java index 053175aa57..95ce33d531 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/NodePermissioningControllerFactoryTest.java @@ -23,6 +23,7 @@ import tech.pegasys.pantheon.ethereum.permissioning.NodeSmartContractPermissioni import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.SmartContractPermissioningConfiguration; import tech.pegasys.pantheon.ethereum.transaction.TransactionSimulator; +import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.util.Collection; @@ -55,7 +56,12 @@ public class NodePermissioningControllerFactoryTest { NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = factory.create( - config, synchronizer, bootnodes, selfEnode.getNodeId(), transactionSimulator); + config, + synchronizer, + bootnodes, + selfEnode.getNodeId(), + transactionSimulator, + new NoOpMetricsSystem()); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(0); @@ -75,7 +81,12 @@ public class NodePermissioningControllerFactoryTest { NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = factory.create( - config, synchronizer, bootnodes, selfEnode.getNodeId(), transactionSimulator); + config, + synchronizer, + bootnodes, + selfEnode.getNodeId(), + transactionSimulator, + new NoOpMetricsSystem()); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(1); @@ -96,7 +107,12 @@ public class NodePermissioningControllerFactoryTest { NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = factory.create( - config, synchronizer, bootnodes, selfEnode.getNodeId(), transactionSimulator); + config, + synchronizer, + bootnodes, + selfEnode.getNodeId(), + transactionSimulator, + new NoOpMetricsSystem()); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(1); @@ -124,7 +140,12 @@ public class NodePermissioningControllerFactoryTest { NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = factory.create( - config, synchronizer, bootnodes, selfEnode.getNodeId(), transactionSimulator); + config, + synchronizer, + bootnodes, + selfEnode.getNodeId(), + transactionSimulator, + new NoOpMetricsSystem()); List providers = controller.getProviders(); assertThat(providers.size()).isEqualTo(2); @@ -155,7 +176,12 @@ public class NodePermissioningControllerFactoryTest { NodePermissioningControllerFactory factory = new NodePermissioningControllerFactory(); NodePermissioningController controller = factory.create( - config, synchronizer, fixedNodes, selfEnode.getNodeId(), transactionSimulator); + config, + synchronizer, + fixedNodes, + selfEnode.getNodeId(), + transactionSimulator, + new NoOpMetricsSystem()); assertThat(controller.getSyncStatusNodePermissioningProvider()).isPresent(); } diff --git a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProviderTest.java b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProviderTest.java index ec5d00618d..94f87e2f45 100644 --- a/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProviderTest.java +++ b/ethereum/permissioning/src/test/java/tech/pegasys/pantheon/ethereum/permissioning/node/provider/SyncStatusNodePermissioningProviderTest.java @@ -14,16 +14,22 @@ package tech.pegasys.pantheon.ethereum.permissioning.node.provider; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import tech.pegasys.pantheon.ethereum.core.SyncStatus; import tech.pegasys.pantheon.ethereum.core.Synchronizer; import tech.pegasys.pantheon.ethereum.core.Synchronizer.SyncStatusListener; +import tech.pegasys.pantheon.metrics.Counter; +import tech.pegasys.pantheon.metrics.MetricCategory; +import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.util.enode.EnodeURL; import java.util.ArrayList; import java.util.Collection; +import java.util.function.Supplier; import org.junit.Before; import org.junit.Test; @@ -34,6 +40,11 @@ import org.mockito.junit.MockitoJUnitRunner; @RunWith(MockitoJUnitRunner.class) public class SyncStatusNodePermissioningProviderTest { + @Mock private MetricsSystem metricsSystem; + @Mock private Counter checkCounter; + @Mock private Counter checkPermittedCounter; + @Mock private Counter checkUnpermittedCounter; + private Supplier syncGauge; private static final EnodeURL bootnode = EnodeURL.fromString( @@ -58,8 +69,34 @@ public class SyncStatusNodePermissioningProviderTest { when(synchronizer.observeSyncStatus(captor.capture())).thenReturn(syncStatusObserverId); bootnodes.add(bootnode); - this.provider = new SyncStatusNodePermissioningProvider(synchronizer, bootnodes); + @SuppressWarnings("unchecked") + final ArgumentCaptor> syncGaugeCallbackCaptor = + ArgumentCaptor.forClass(Supplier.class); + + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "sync_status_node_check_count", + "Number of times the sync status permissioning provider has been checked")) + .thenReturn(checkCounter); + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "sync_status_node_check_count_permitted", + "Number of times the sync status permissioning provider has been checked and returned permitted")) + .thenReturn(checkPermittedCounter); + when(metricsSystem.createCounter( + MetricCategory.PERMISSIONING, + "sync_status_node_check_count_unpermitted", + "Number of times the sync status permissioning provider has been checked and returned unpermitted")) + .thenReturn(checkUnpermittedCounter); + this.provider = new SyncStatusNodePermissioningProvider(synchronizer, bootnodes, metricsSystem); this.syncStatusListener = captor.getValue(); + verify(metricsSystem) + .createIntegerGauge( + eq(MetricCategory.PERMISSIONING), + eq("sync_status_node_sync_reached"), + eq("Whether the sync status permissioning provider has realised sync yet"), + syncGaugeCallbackCaptor.capture()); + this.syncGauge = syncGaugeCallbackCaptor.getValue(); verify(synchronizer).observeSyncStatus(any()); } @@ -69,6 +106,7 @@ public class SyncStatusNodePermissioningProviderTest { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 2)); assertThat(provider.hasReachedSync()).isFalse(); + assertThat(syncGauge.get()).isEqualTo(0); } @Test @@ -76,57 +114,77 @@ public class SyncStatusNodePermissioningProviderTest { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 1)); assertThat(provider.hasReachedSync()).isTrue(); + assertThat(syncGauge.get()).isEqualTo(1); } @Test public void whenInSyncChangesFromTrueToFalseHasReachedSyncShouldReturnTrue() { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 2)); assertThat(provider.hasReachedSync()).isFalse(); + assertThat(syncGauge.get()).isEqualTo(0); syncStatusListener.onSyncStatus(new SyncStatus(0, 2, 1)); assertThat(provider.hasReachedSync()).isTrue(); + assertThat(syncGauge.get()).isEqualTo(1); syncStatusListener.onSyncStatus(new SyncStatus(0, 2, 3)); assertThat(provider.hasReachedSync()).isTrue(); + assertThat(syncGauge.get()).isEqualTo(1); } @Test public void whenHasNotSyncedNonBootnodeShouldNotBePermitted() { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 2)); assertThat(provider.hasReachedSync()).isFalse(); + assertThat(syncGauge.get()).isEqualTo(0); boolean isPermitted = provider.isPermitted(enode1, enode2); assertThat(isPermitted).isFalse(); + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(1)).inc(); } @Test public void whenHasNotSyncedBootnodeIncomingConnectionShouldNotBePermitted() { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 2)); assertThat(provider.hasReachedSync()).isFalse(); + assertThat(syncGauge.get()).isEqualTo(0); boolean isPermitted = provider.isPermitted(bootnode, enode1); assertThat(isPermitted).isFalse(); + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(1)).inc(); } @Test public void whenHasNotSyncedBootnodeOutgoingConnectionShouldBePermitted() { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 2)); assertThat(provider.hasReachedSync()).isFalse(); + assertThat(syncGauge.get()).isEqualTo(0); boolean isPermitted = provider.isPermitted(enode1, bootnode); assertThat(isPermitted).isTrue(); + verify(checkCounter, times(1)).inc(); + verify(checkPermittedCounter, times(1)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); } @Test public void whenHasSyncedIsPermittedShouldReturnTrue() { syncStatusListener.onSyncStatus(new SyncStatus(0, 1, 1)); assertThat(provider.hasReachedSync()).isTrue(); + assertThat(syncGauge.get()).isEqualTo(1); boolean isPermitted = provider.isPermitted(enode1, enode2); assertThat(isPermitted).isTrue(); + verify(checkCounter, times(0)).inc(); + verify(checkPermittedCounter, times(0)).inc(); + verify(checkUnpermittedCounter, times(0)).inc(); } } diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java index 0f6c81b27b..f203b0b03c 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java @@ -23,6 +23,7 @@ public enum MetricCategory { NETWORK("network"), PEERS("peers"), PROCESS("process", false), + PERMISSIONING("permissioning"), KVSTORE_ROCKSDB("rocksdb"), KVSTORE_ROCKSDB_STATS("rocksdb", false), RPC("rpc"), diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java index f9d14e057a..d469561881 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/RunnerBuilder.java @@ -427,7 +427,13 @@ public class RunnerBuilder { return permissioningConfiguration.map( config -> new NodePermissioningControllerFactory() - .create(config, synchronizer, fixedNodes, localNodeId, transactionSimulator)); + .create( + config, + synchronizer, + fixedNodes, + localNodeId, + transactionSimulator, + metricsSystem)); } @VisibleForTesting