From 927db60fe9ad8d54fe7733f5c35b19d8d5c4efe7 Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Fri, 7 May 2021 13:14:52 -0500 Subject: [PATCH 1/9] Add signature download consensus ratio option and msising nodes metric Signed-off-by: Nana-EC --- docs/configuration.md | 3 + .../importer/downloader/Downloader.java | 16 ------ .../downloader/DownloaderProperties.java | 2 + .../downloader/NodeSignatureVerifier.java | 57 ++++++++++++++++--- .../balance/BalanceDownloaderProperties.java | 5 ++ .../event/EventDownloaderProperties.java | 5 ++ .../record/RecordDownloaderProperties.java | 5 ++ .../downloader/AbstractDownloaderTest.java | 2 +- .../downloader/NodeSignatureVerifierTest.java | 11 +++- 9 files changed, 79 insertions(+), 27 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index c5d3bf03efd..79f37815002 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -36,6 +36,7 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.accessKey` | "" | The cloud storage access key | | `hedera.mirror.importer.downloader.allowAnonymousAccess` | | Whether the cloud storage bucket allows for anonymous access. | | `hedera.mirror.importer.downloader.balance.batchSize` | 30 | The number of signature files to download per node before downloading the signed files | +| `hedera.mirror.importer.downloader.balance.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature files | | `hedera.mirror.importer.downloader.balance.enabled` | true | Whether to enable balance file downloads | | `hedera.mirror.importer.downloader.balance.frequency` | 30s | The fixed period between invocations. Can accept duration units like `10s`, `2m`, etc. | | `hedera.mirror.importer.downloader.balance.keepSignatures` | false | Whether to keep balance signature files after successful verification. If false, files are deleted. | @@ -45,6 +46,7 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.cloudProvider` | S3 | The cloud provider to download files from. Either `S3` or `GCP` | | `hedera.mirror.importer.downloader.endpointOverride` | | Can be specified to download streams from a source other than S3 and GCP. Should be S3 compatible | | `hedera.mirror.importer.downloader.event.batchSize` | 100 | The number of signature files to download per node before downloading the signed files | +| `hedera.mirror.importer.downloader.event.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature files | | `hedera.mirror.importer.downloader.event.enabled` | false | Whether to enable event file downloads | | `hedera.mirror.importer.downloader.event.frequency` | 5s | The fixed period between invocations. Can accept duration units like `10s`, `2m`, etc. | | `hedera.mirror.importer.downloader.event.keepSignatures` | false | Whether to keep event signature files after successful verification. If false, files are deleted. | @@ -53,6 +55,7 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.gcpProjectId` | | GCP project id to bill for requests to GCS bucket which has Requester Pays enabled. | | `hedera.mirror.importer.downloader.maxConcurrency` | 1000 | The maximum number of allowed open HTTP connections. Used by AWS SDK directly. | | `hedera.mirror.importer.downloader.record.batchSize` | 40 | The number of signature files to download per node before downloading the signed files | +| `hedera.mirror.importer.downloader.record.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature files | | `hedera.mirror.importer.downloader.record.enabled` | true | Whether to enable record file downloads | | `hedera.mirror.importer.downloader.record.frequency` | 500ms | The fixed period between invocations. Can accept duration units like `10s`, `2m`, etc. | | `hedera.mirror.importer.downloader.record.keepSignatures` | false | Whether to keep record signature files after successful verification. If false, files are deleted. | diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java index 4e5b89f219b..9b1edf2d3b4 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java @@ -29,7 +29,6 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; import com.google.common.collect.TreeMultimap; -import io.micrometer.core.instrument.Counter; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Timer; import java.nio.file.Path; @@ -99,7 +98,6 @@ public abstract class Downloader { private final MeterRegistry meterRegistry; private final Timer cloudStorageLatencyMetric; private final Timer downloadLatencyMetric; - private final Counter.Builder signatureVerificationMetric; private final Timer streamCloseMetric; private final Timer.Builder streamVerificationMetric; @@ -138,10 +136,6 @@ protected Downloader(S3AsyncClient s3Client, .tag("type", streamType.toString()) .register(meterRegistry); - signatureVerificationMetric = Counter.builder("hedera.mirror.download.signature.verification") - .description("The number of signatures verified from a particular node") - .tag("type", streamType.toString()); - streamCloseMetric = Timer.builder("hedera.mirror.stream.close.latency") .description("The difference between the consensus time of the last and first transaction in the " + "stream file") @@ -395,16 +389,6 @@ private void verifySigsAndDownloadDataFiles(Multimap= Math.ceil(expectedNodes / 3.0); + private static boolean canReachConsensus(long actualNodes, long expectedNodes, double consensusRatio) { + return actualNodes >= Math.ceil(expectedNodes * consensusRatio); } /** @@ -75,7 +98,7 @@ public void verify(Collection signatures) throws SignatureV long sigFileCount = signatures.size(); long nodeCount = nodeAccountIDPubKeyMap.size(); - if (!canReachConsensus(sigFileCount, nodeCount)) { + if (!canReachConsensus(sigFileCount, nodeCount, downloaderProperties.getConsensusRatio())) { throw new SignatureVerificationException("Require at least 1/3 signature files to reach consensus, got " + sigFileCount + " out of " + nodeCount + " for file " + filename + ": " + statusMap(signatures, nodeAccountIDPubKeyMap)); @@ -91,7 +114,7 @@ public void verify(Collection signatures) throws SignatureV for (String key : signatureHashMap.keySet()) { Collection validatedSignatures = signatureHashMap.get(key); - if (canReachConsensus(validatedSignatures.size(), nodeCount)) { + if (canReachConsensus(validatedSignatures.size(), nodeCount, downloaderProperties.getConsensusRatio())) { consensusCount += validatedSignatures.size(); validatedSignatures.forEach(s -> s.setStatus(SignatureStatus.CONSENSUS_REACHED)); } @@ -159,11 +182,29 @@ private Map> statusMap(Collection fss.getStatus().toString(), Collectors.mapping(FileStreamSignature::getNodeAccountIdString, Collectors .toCollection(TreeSet::new)))); - Set seenNodes = signatures.stream().map(FileStreamSignature::getNodeAccountIdString) - .collect(Collectors.toSet()); + + Set seenNodes = new HashSet<>(); + signatures.forEach(signature -> { + seenNodes.add(signature.getNodeAccountId().entityIdToString()); + EntityId nodeAccountId = signature.getNodeAccountId(); + signatureVerificationMetric.tag("nodeAccount", nodeAccountId.getEntityNum().toString()) + .tag("realm", nodeAccountId.getRealmNum().toString()) + .tag("shard", nodeAccountId.getShardNum().toString()) + .tag("status", signature.getStatus().toString()) + .register(meterRegistry) + .increment(); + }); + Set missingNodes = new TreeSet<>(Sets.difference(nodeAccountIDPubKeyMap.keySet(), seenNodes)); statusMap.put("MISSING", missingNodes); statusMap.remove(SignatureStatus.CONSENSUS_REACHED.toString()); + + missingNodes.forEach(nodeAccountId -> { + missingNodeSignatureFileMetric.tag("nodeAccount", nodeAccountId) + .register(meterRegistry) + .increment(); + }); + return statusMap; } } diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java index 4385ab996a1..b26514cb221 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java @@ -22,6 +22,7 @@ import java.nio.file.Path; import java.time.Duration; +import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -46,6 +47,10 @@ public class BalanceDownloaderProperties implements DownloaderProperties { @Min(1) private int batchSize = 30; + @Max(1) + @Min(0) + private double consensusRatio = 0.333; + private boolean enabled = true; @NotNull diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java index 1b5bbdc98e3..34c4020ea5d 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java @@ -22,6 +22,7 @@ import java.nio.file.Path; import java.time.Duration; +import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -46,6 +47,10 @@ public class EventDownloaderProperties implements DownloaderProperties { @Min(1) private int batchSize = 100; + @Max(1) + @Min(0) + private double consensusRatio = 0.333; + private boolean enabled = false; @NotNull diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java index 9873c9ac6ed..6b1a86bd72e 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java @@ -22,6 +22,7 @@ import java.nio.file.Path; import java.time.Duration; +import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -46,6 +47,10 @@ public class RecordDownloaderProperties implements DownloaderProperties { @Min(1) private int batchSize = 40; + @Max(1) + @Min(0) + private double consensusRatio = 0.333; + private boolean enabled = true; @NotNull diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java index 2a367e28284..66841af0765 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java @@ -200,7 +200,7 @@ mirrorProperties, commonDownloaderProperties, new MetricsExecutionInterceptor(me signatureFileReader = new CompositeSignatureFileReader(new SignatureFileReaderV2(), new SignatureFileReaderV5()); - nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService); + nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, downloaderProperties, meterRegistry); downloader = getDownloader(); streamType = downloaderProperties.getStreamType(); diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java index 03e74beb620..ea72b2ff6ff 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java @@ -23,6 +23,8 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.when; +import io.micrometer.core.instrument.MeterRegistry; +import io.micrometer.core.instrument.logging.LoggingMeterRegistry; import java.security.GeneralSecurityException; import java.security.KeyPair; import java.security.KeyPairGenerator; @@ -57,11 +59,15 @@ class NodeSignatureVerifierTest { private static PublicKey publicKey; private static final EntityId nodeId = new EntityId(0L, 0L, 3L, EntityTypeEnum.ACCOUNT.getId()); + private static final MeterRegistry meterRegistry = new LoggingMeterRegistry(); private Signature signer; @Mock private AddressBookService addressBookService; + @Mock + private DownloaderProperties downloaderProperties; + @Mock private AddressBook currentAddressBook; @@ -76,7 +82,7 @@ static void generateKeys() throws NoSuchAlgorithmException { @BeforeEach void setup() throws GeneralSecurityException { - nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService); + nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, downloaderProperties, meterRegistry); signer = Signature.getInstance("SHA384withRSA", "SunRsaSign"); signer.initSign(privateKey); Map nodeAccountIDPubKeyMap = new HashMap(); @@ -174,7 +180,8 @@ void testVerifiedWithOneThirdConsensus() throws GeneralSecurityException { null, null); fileStreamSignatureNode4.setNodeAccountId(new EntityId(0L, 0L, 5L, EntityTypeEnum.ACCOUNT.getId())); - nodeSignatureVerifier.verify(Arrays.asList(fileStreamSignatureNode3, fileStreamSignatureNode5)); + nodeSignatureVerifier + .verify(Arrays.asList(fileStreamSignatureNode3, fileStreamSignatureNode5)); } @Test From aca197f3f11c309365f947a167e161f9ac42f2d5 Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Fri, 7 May 2021 13:31:21 -0500 Subject: [PATCH 2/9] Fix missingNodeSignatureFileMetric description Signed-off-by: Nana-EC --- .../mirror/importer/downloader/NodeSignatureVerifier.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index a5bcf60cf0e..43c3cb8b191 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -63,7 +63,7 @@ public NodeSignatureVerifier(AddressBookService addressBookService, DownloaderPr String streamType = downloaderProperties.getStreamType().toString(); missingNodeSignatureFileMetric = Counter.builder("hedera.mirror.download.signature.missing") - .description("The number of signatures verified from a particular node") + .description("The number of nodes whose signatures are missing from the consensus process") .tag("type", streamType); signatureVerificationMetric = Counter.builder("hedera.mirror.download.signature.verification") .description("The number of signatures verified from a particular node") From 94e97ee65060531ac31881efc30e387859fb77a3 Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Fri, 7 May 2021 19:19:37 -0500 Subject: [PATCH 3/9] Simplify by moving consensusRatio to commonDownloaderProperties Signed-off-by: Nana-EC --- docs/configuration.md | 4 +--- .../importer/domain/FileStreamSignature.java | 1 + .../CommonDownloaderProperties.java | 5 +++++ .../importer/downloader/Downloader.java | 1 + .../downloader/DownloaderProperties.java | 2 -- .../downloader/NodeSignatureVerifier.java | 22 ++++++++++--------- .../balance/BalanceDownloaderProperties.java | 5 ----- .../event/EventDownloaderProperties.java | 5 ----- .../record/RecordDownloaderProperties.java | 5 ----- .../downloader/AbstractDownloaderTest.java | 3 ++- .../downloader/NodeSignatureVerifierTest.java | 8 +++++-- 11 files changed, 28 insertions(+), 33 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 79f37815002..53edf7929b2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -36,7 +36,6 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.accessKey` | "" | The cloud storage access key | | `hedera.mirror.importer.downloader.allowAnonymousAccess` | | Whether the cloud storage bucket allows for anonymous access. | | `hedera.mirror.importer.downloader.balance.batchSize` | 30 | The number of signature files to download per node before downloading the signed files | -| `hedera.mirror.importer.downloader.balance.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature files | | `hedera.mirror.importer.downloader.balance.enabled` | true | Whether to enable balance file downloads | | `hedera.mirror.importer.downloader.balance.frequency` | 30s | The fixed period between invocations. Can accept duration units like `10s`, `2m`, etc. | | `hedera.mirror.importer.downloader.balance.keepSignatures` | false | Whether to keep balance signature files after successful verification. If false, files are deleted. | @@ -44,9 +43,9 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.balance.threads` | 15 | The number of threads to search for new files to download | | `hedera.mirror.importer.downloader.bucketName` | | The cloud storage bucket name to download streamed files. This value takes priority over network hardcoded bucket names regardless of `hedera.mirror.importer.network` value.| | `hedera.mirror.importer.downloader.cloudProvider` | S3 | The cloud provider to download files from. Either `S3` or `GCP` | +| `hedera.mirror.importer.downloader.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature file hash | | `hedera.mirror.importer.downloader.endpointOverride` | | Can be specified to download streams from a source other than S3 and GCP. Should be S3 compatible | | `hedera.mirror.importer.downloader.event.batchSize` | 100 | The number of signature files to download per node before downloading the signed files | -| `hedera.mirror.importer.downloader.event.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature files | | `hedera.mirror.importer.downloader.event.enabled` | false | Whether to enable event file downloads | | `hedera.mirror.importer.downloader.event.frequency` | 5s | The fixed period between invocations. Can accept duration units like `10s`, `2m`, etc. | | `hedera.mirror.importer.downloader.event.keepSignatures` | false | Whether to keep event signature files after successful verification. If false, files are deleted. | @@ -55,7 +54,6 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.gcpProjectId` | | GCP project id to bill for requests to GCS bucket which has Requester Pays enabled. | | `hedera.mirror.importer.downloader.maxConcurrency` | 1000 | The maximum number of allowed open HTTP connections. Used by AWS SDK directly. | | `hedera.mirror.importer.downloader.record.batchSize` | 40 | The number of signature files to download per node before downloading the signed files | -| `hedera.mirror.importer.downloader.record.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature files | | `hedera.mirror.importer.downloader.record.enabled` | true | Whether to enable record file downloads | | `hedera.mirror.importer.downloader.record.frequency` | 500ms | The fixed period between invocations. Can accept duration units like `10s`, `2m`, etc. | | `hedera.mirror.importer.downloader.record.keepSignatures` | false | Whether to keep record signature files after successful verification. If false, files are deleted. | diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java index 9e084eec696..0970cd7f5c3 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java @@ -45,6 +45,7 @@ public class FileStreamSignature implements Comparable { private SignatureStatus status = SignatureStatus.DOWNLOADED; private byte[] metadataHash; private byte[] metadataHashSignature; + private StreamType streamType; @Override public int compareTo(FileStreamSignature other) { diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java index aa5f5a544bf..1abcde9d5d1 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java @@ -20,6 +20,7 @@ * ‍ */ +import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotNull; import lombok.Data; @@ -46,6 +47,10 @@ public String getBucketName() { return StringUtils.isNotBlank(bucketName) ? bucketName : mirrorProperties.getNetwork().getBucketName(); } + @Max(1) + @Min(0) + private double consensusRatio = 0.333; + @NotNull private CloudProvider cloudProvider = CloudProvider.S3; diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java index 9b1edf2d3b4..9afc0073aa3 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java @@ -307,6 +307,7 @@ private Optional parseSignatureFile(PendingDownload pending StreamFileData streamFileData = new StreamFileData(streamFilename, pendingDownload.getBytes()); FileStreamSignature fileStreamSignature = signatureFileReader.read(streamFileData); fileStreamSignature.setNodeAccountId(nodeAccountId); + fileStreamSignature.setStreamType(pendingDownload.getStreamFilename().getStreamType()); return Optional.of(fileStreamSignature); } diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/DownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/DownloaderProperties.java index 0458f02f186..94dd6adca80 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/DownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/DownloaderProperties.java @@ -31,8 +31,6 @@ public interface DownloaderProperties { CommonDownloaderProperties getCommon(); - double getConsensusRatio(); - MirrorProperties getMirrorProperties(); /** diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index 43c3cb8b191..1fc5f6b4ee5 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -48,26 +48,24 @@ public class NodeSignatureVerifier { private final AddressBookService addressBookService; - private final DownloaderProperties downloaderProperties; + private final CommonDownloaderProperties commonDownloaderProperties; // Metrics private final MeterRegistry meterRegistry; private final Counter.Builder missingNodeSignatureFileMetric; private final Counter.Builder signatureVerificationMetric; - public NodeSignatureVerifier(AddressBookService addressBookService, DownloaderProperties downloaderProperties, + public NodeSignatureVerifier(AddressBookService addressBookService, + CommonDownloaderProperties commonDownloaderProperties, MeterRegistry meterRegistry) { this.addressBookService = addressBookService; - this.downloaderProperties = downloaderProperties; + this.commonDownloaderProperties = commonDownloaderProperties; this.meterRegistry = meterRegistry; - String streamType = downloaderProperties.getStreamType().toString(); missingNodeSignatureFileMetric = Counter.builder("hedera.mirror.download.signature.missing") - .description("The number of nodes whose signatures are missing from the consensus process") - .tag("type", streamType); + .description("The number of nodes whose signatures are missing from the consensus process"); signatureVerificationMetric = Counter.builder("hedera.mirror.download.signature.verification") - .description("The number of signatures verified from a particular node") - .tag("type", streamType); + .description("The number of signatures verified from a particular node"); } private static boolean canReachConsensus(long actualNodes, long expectedNodes, double consensusRatio) { @@ -98,7 +96,7 @@ public void verify(Collection signatures) throws SignatureV long sigFileCount = signatures.size(); long nodeCount = nodeAccountIDPubKeyMap.size(); - if (!canReachConsensus(sigFileCount, nodeCount, downloaderProperties.getConsensusRatio())) { + if (!canReachConsensus(sigFileCount, nodeCount, commonDownloaderProperties.getConsensusRatio())) { throw new SignatureVerificationException("Require at least 1/3 signature files to reach consensus, got " + sigFileCount + " out of " + nodeCount + " for file " + filename + ": " + statusMap(signatures, nodeAccountIDPubKeyMap)); @@ -114,7 +112,8 @@ public void verify(Collection signatures) throws SignatureV for (String key : signatureHashMap.keySet()) { Collection validatedSignatures = signatureHashMap.get(key); - if (canReachConsensus(validatedSignatures.size(), nodeCount, downloaderProperties.getConsensusRatio())) { + if (canReachConsensus(validatedSignatures.size(), nodeCount, commonDownloaderProperties + .getConsensusRatio())) { consensusCount += validatedSignatures.size(); validatedSignatures.forEach(s -> s.setStatus(SignatureStatus.CONSENSUS_REACHED)); } @@ -191,6 +190,7 @@ private Map> statusMap(Collection> statusMap(Collection { missingNodeSignatureFileMetric.tag("nodeAccount", nodeAccountId) + .tag("type", streamType) .register(meterRegistry) .increment(); }); diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java index b26514cb221..4385ab996a1 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/balance/BalanceDownloaderProperties.java @@ -22,7 +22,6 @@ import java.nio.file.Path; import java.time.Duration; -import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -47,10 +46,6 @@ public class BalanceDownloaderProperties implements DownloaderProperties { @Min(1) private int batchSize = 30; - @Max(1) - @Min(0) - private double consensusRatio = 0.333; - private boolean enabled = true; @NotNull diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java index 34c4020ea5d..1b5bbdc98e3 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/event/EventDownloaderProperties.java @@ -22,7 +22,6 @@ import java.nio.file.Path; import java.time.Duration; -import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -47,10 +46,6 @@ public class EventDownloaderProperties implements DownloaderProperties { @Min(1) private int batchSize = 100; - @Max(1) - @Min(0) - private double consensusRatio = 0.333; - private boolean enabled = false; @NotNull diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java index 6b1a86bd72e..9873c9ac6ed 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/record/RecordDownloaderProperties.java @@ -22,7 +22,6 @@ import java.nio.file.Path; import java.time.Duration; -import javax.validation.constraints.Max; import javax.validation.constraints.Min; import javax.validation.constraints.NotBlank; import javax.validation.constraints.NotNull; @@ -47,10 +46,6 @@ public class RecordDownloaderProperties implements DownloaderProperties { @Min(1) private int batchSize = 40; - @Max(1) - @Min(0) - private double consensusRatio = 0.333; - private boolean enabled = true; @NotNull diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java index 66841af0765..308ee104dbe 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java @@ -200,7 +200,8 @@ mirrorProperties, commonDownloaderProperties, new MetricsExecutionInterceptor(me signatureFileReader = new CompositeSignatureFileReader(new SignatureFileReaderV2(), new SignatureFileReaderV5()); - nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, downloaderProperties, meterRegistry); + nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, downloaderProperties + .getCommon(), meterRegistry); downloader = getDownloader(); streamType = downloaderProperties.getStreamType(); diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java index ea72b2ff6ff..b3dfdf9f1f9 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java @@ -50,6 +50,7 @@ import com.hedera.mirror.importer.domain.EntityTypeEnum; import com.hedera.mirror.importer.domain.FileStreamSignature; import com.hedera.mirror.importer.domain.FileStreamSignature.SignatureType; +import com.hedera.mirror.importer.domain.StreamType; import com.hedera.mirror.importer.exception.SignatureVerificationException; @ExtendWith(MockitoExtension.class) @@ -66,7 +67,7 @@ class NodeSignatureVerifierTest { private AddressBookService addressBookService; @Mock - private DownloaderProperties downloaderProperties; + private CommonDownloaderProperties commonDownloaderProperties; @Mock private AddressBook currentAddressBook; @@ -82,13 +83,15 @@ static void generateKeys() throws NoSuchAlgorithmException { @BeforeEach void setup() throws GeneralSecurityException { - nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, downloaderProperties, meterRegistry); + nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, commonDownloaderProperties, + meterRegistry); signer = Signature.getInstance("SHA384withRSA", "SunRsaSign"); signer.initSign(privateKey); Map nodeAccountIDPubKeyMap = new HashMap(); nodeAccountIDPubKeyMap.put("0.0.3", publicKey); when(addressBookService.getCurrent()).thenReturn(currentAddressBook); when(currentAddressBook.getNodeAccountIDPubKeyMap()).thenReturn(nodeAccountIDPubKeyMap); + when(commonDownloaderProperties.getConsensusRatio()).thenReturn(0.333); } @Test @@ -266,6 +269,7 @@ private FileStreamSignature buildBareBonesFileStreamSignature() { fileStreamSignature.setFilename(""); fileStreamSignature.setNodeAccountId(nodeId); fileStreamSignature.setSignatureType(SignatureType.SHA_384_WITH_RSA); + fileStreamSignature.setStreamType(StreamType.RECORD); return fileStreamSignature; } From 52b06b1be5ece91e63ff249355ca30506f16d898 Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Fri, 7 May 2021 19:26:39 -0500 Subject: [PATCH 4/9] Cleaned up a bit Signed-off-by: Nana-EC --- .../importer/downloader/NodeSignatureVerifier.java | 10 ++++------ .../importer/downloader/AbstractDownloaderTest.java | 6 ++++-- .../importer/downloader/NodeSignatureVerifierTest.java | 4 +++- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index 1fc5f6b4ee5..bf13841b63d 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -200,12 +200,10 @@ private Map> statusMap(Collection { - missingNodeSignatureFileMetric.tag("nodeAccount", nodeAccountId) - .tag("type", streamType) - .register(meterRegistry) - .increment(); - }); + missingNodes.forEach(nodeAccountId -> missingNodeSignatureFileMetric.tag("nodeAccount", nodeAccountId) + .tag("type", streamType) + .register(meterRegistry) + .increment()); return statusMap; } diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java index 308ee104dbe..0fc52b57b05 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/AbstractDownloaderTest.java @@ -200,8 +200,10 @@ mirrorProperties, commonDownloaderProperties, new MetricsExecutionInterceptor(me signatureFileReader = new CompositeSignatureFileReader(new SignatureFileReaderV2(), new SignatureFileReaderV5()); - nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, downloaderProperties - .getCommon(), meterRegistry); + nodeSignatureVerifier = new NodeSignatureVerifier( + addressBookService, + downloaderProperties.getCommon(), + meterRegistry); downloader = getDownloader(); streamType = downloaderProperties.getStreamType(); diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java index b3dfdf9f1f9..f0bfe2543c4 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java @@ -83,7 +83,9 @@ static void generateKeys() throws NoSuchAlgorithmException { @BeforeEach void setup() throws GeneralSecurityException { - nodeSignatureVerifier = new NodeSignatureVerifier(addressBookService, commonDownloaderProperties, + nodeSignatureVerifier = new NodeSignatureVerifier( + addressBookService, + commonDownloaderProperties, meterRegistry); signer = Signature.getInstance("SHA384withRSA", "SunRsaSign"); signer.initSign(privateKey); From 5e3414417e28239f5cc27c82072d8b96d2003800 Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Wed, 12 May 2021 17:04:11 -0500 Subject: [PATCH 5/9] Addressed feedback Signed-off-by: Nana-EC --- docs/configuration.md | 2 +- .../CommonDownloaderProperties.java | 2 +- .../importer/downloader/Downloader.java | 2 +- .../downloader/NodeSignatureVerifier.java | 56 ++++++++++++------- .../downloader/NodeSignatureVerifierTest.java | 54 +++++++++++++++++- 5 files changed, 92 insertions(+), 24 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 53edf7929b2..7af74cd2563 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -43,7 +43,7 @@ value, it is recommended to only populate overridden properties in the custom `a | `hedera.mirror.importer.downloader.balance.threads` | 15 | The number of threads to search for new files to download | | `hedera.mirror.importer.downloader.bucketName` | | The cloud storage bucket name to download streamed files. This value takes priority over network hardcoded bucket names regardless of `hedera.mirror.importer.network` value.| | `hedera.mirror.importer.downloader.cloudProvider` | S3 | The cloud provider to download files from. Either `S3` or `GCP` | -| `hedera.mirror.importer.downloader.consensusRatio` | 0.333 | The ratio of verified nodes to total number of nodes available used to come to consensus on the signature file hash | +| `hedera.mirror.importer.downloader.consensusRatio` | 0.333 | The ratio of verified nodes (nodes used to come to consensus on the signature file hash) to total number of nodes available | | `hedera.mirror.importer.downloader.endpointOverride` | | Can be specified to download streams from a source other than S3 and GCP. Should be S3 compatible | | `hedera.mirror.importer.downloader.event.batchSize` | 100 | The number of signature files to download per node before downloading the signed files | | `hedera.mirror.importer.downloader.event.enabled` | false | Whether to enable event file downloads | diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java index 1abcde9d5d1..52ddc1fd359 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/CommonDownloaderProperties.java @@ -49,7 +49,7 @@ public String getBucketName() { @Max(1) @Min(0) - private double consensusRatio = 0.333; + private float consensusRatio = 0.333f; @NotNull private CloudProvider cloudProvider = CloudProvider.S3; diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java index 9afc0073aa3..b19a5e513de 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/Downloader.java @@ -307,7 +307,7 @@ private Optional parseSignatureFile(PendingDownload pending StreamFileData streamFileData = new StreamFileData(streamFilename, pendingDownload.getBytes()); FileStreamSignature fileStreamSignature = signatureFileReader.read(streamFileData); fileStreamSignature.setNodeAccountId(nodeAccountId); - fileStreamSignature.setStreamType(pendingDownload.getStreamFilename().getStreamType()); + fileStreamSignature.setStreamType(streamType); return Optional.of(fileStreamSignature); } diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index bf13841b63d..4deb923936a 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -32,6 +32,7 @@ import java.util.Map; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import javax.inject.Named; import lombok.extern.log4j.Log4j2; @@ -39,6 +40,7 @@ import com.hedera.mirror.importer.addressbook.AddressBookService; import com.hedera.mirror.importer.domain.AddressBook; import com.hedera.mirror.importer.domain.EntityId; +import com.hedera.mirror.importer.domain.EntityTypeEnum; import com.hedera.mirror.importer.domain.FileStreamSignature; import com.hedera.mirror.importer.domain.FileStreamSignature.SignatureStatus; import com.hedera.mirror.importer.exception.SignatureVerificationException; @@ -52,6 +54,7 @@ public class NodeSignatureVerifier { // Metrics private final MeterRegistry meterRegistry; + private final Map metricMap = new ConcurrentHashMap<>(); private final Counter.Builder missingNodeSignatureFileMetric; private final Counter.Builder signatureVerificationMetric; @@ -68,8 +71,8 @@ public NodeSignatureVerifier(AddressBookService addressBookService, .description("The number of signatures verified from a particular node"); } - private static boolean canReachConsensus(long actualNodes, long expectedNodes, double consensusRatio) { - return actualNodes >= Math.ceil(expectedNodes * consensusRatio); + private boolean canReachConsensus(long actualNodes, long expectedNodes) { + return actualNodes >= Math.ceil(expectedNodes * commonDownloaderProperties.getConsensusRatio()); } /** @@ -96,10 +99,14 @@ public void verify(Collection signatures) throws SignatureV long sigFileCount = signatures.size(); long nodeCount = nodeAccountIDPubKeyMap.size(); - if (!canReachConsensus(sigFileCount, nodeCount, commonDownloaderProperties.getConsensusRatio())) { - throw new SignatureVerificationException("Require at least 1/3 signature files to reach consensus, got " + - sigFileCount + " out of " + nodeCount + " for file " + filename + ": " + statusMap(signatures, - nodeAccountIDPubKeyMap)); + if (!canReachConsensus(sigFileCount, nodeCount)) { + throw new SignatureVerificationException(String.format( + "Requires at least %s of signature files to reach consensus, got %d out of %d for file %s: %s", + String.format("%.03f", commonDownloaderProperties.getConsensusRatio()), + sigFileCount, + nodeCount, + filename, + statusMap(signatures, nodeAccountIDPubKeyMap))); } for (FileStreamSignature fileStreamSignature : signatures) { @@ -112,8 +119,7 @@ public void verify(Collection signatures) throws SignatureV for (String key : signatureHashMap.keySet()) { Collection validatedSignatures = signatureHashMap.get(key); - if (canReachConsensus(validatedSignatures.size(), nodeCount, commonDownloaderProperties - .getConsensusRatio())) { + if (canReachConsensus(validatedSignatures.size(), nodeCount)) { consensusCount += validatedSignatures.size(); validatedSignatures.forEach(s -> s.setStatus(SignatureStatus.CONSENSUS_REACHED)); } @@ -126,6 +132,9 @@ public void verify(Collection signatures) throws SignatureV log.warn("Verified signature file {} reached consensus but with some errors: {}", filename, statusMap(signatures, nodeAccountIDPubKeyMap)); return; + } else if (commonDownloaderProperties.getConsensusRatio() == 0) { + log.debug("Signature file {} does not require consensus, skipping verification", filename); + return; } throw new SignatureVerificationException("Signature verification failed for file " + filename + ": " + statusMap(signatures, nodeAccountIDPubKeyMap)); @@ -175,34 +184,43 @@ private boolean verifySignature(FileStreamSignature fileStreamSignature, return false; } - private Map> statusMap(Collection signatures, Map> statusMap(Collection signatures, Map nodeAccountIDPubKeyMap) { - Map> statusMap = signatures.stream() + Map> statusMap = signatures.stream() .collect(Collectors.groupingBy(fss -> fss.getStatus().toString(), - Collectors.mapping(FileStreamSignature::getNodeAccountIdString, Collectors + Collectors.mapping(FileStreamSignature::getNodeAccountId, Collectors .toCollection(TreeSet::new)))); - Set seenNodes = new HashSet<>(); + Set seenNodes = new HashSet<>(); signatures.forEach(signature -> { - seenNodes.add(signature.getNodeAccountId().entityIdToString()); + seenNodes.add(signature.getNodeAccountId()); EntityId nodeAccountId = signature.getNodeAccountId(); - signatureVerificationMetric.tag("nodeAccount", nodeAccountId.getEntityNum().toString()) + + metricMap.computeIfAbsent(nodeAccountId, n -> signatureVerificationMetric + .tag("nodeAccount", nodeAccountId.getEntityNum().toString()) .tag("realm", nodeAccountId.getRealmNum().toString()) .tag("shard", nodeAccountId.getShardNum().toString()) .tag("status", signature.getStatus().toString()) .tag("type", signature.getStreamType().toString()) - .register(meterRegistry) + .register(meterRegistry)) .increment(); }); - Set missingNodes = new TreeSet<>(Sets.difference(nodeAccountIDPubKeyMap.keySet(), seenNodes)); + Set missingNodes = new TreeSet<>(Sets.difference( + nodeAccountIDPubKeyMap.keySet().stream().map(x -> EntityId.of(x, EntityTypeEnum.ACCOUNT)) + .collect(Collectors.toSet()), + seenNodes)); statusMap.put("MISSING", missingNodes); statusMap.remove(SignatureStatus.CONSENSUS_REACHED.toString()); String streamType = signatures.stream().map(FileStreamSignature::getStreamType).findFirst().toString(); - missingNodes.forEach(nodeAccountId -> missingNodeSignatureFileMetric.tag("nodeAccount", nodeAccountId) - .tag("type", streamType) - .register(meterRegistry) + missingNodes.forEach(nodeAccountId -> metricMap + .computeIfAbsent(nodeAccountId, n -> missingNodeSignatureFileMetric + .tag("nodeAccount", n.getEntityNum().toString()) + .tag("realm", nodeAccountId.getRealmNum().toString()) + .tag("shard", nodeAccountId.getShardNum().toString()) + .tag("type", streamType) + .register(meterRegistry)) .increment()); return statusMap; diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java index f0bfe2543c4..a7bf39e97f6 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java @@ -93,7 +93,7 @@ void setup() throws GeneralSecurityException { nodeAccountIDPubKeyMap.put("0.0.3", publicKey); when(addressBookService.getCurrent()).thenReturn(currentAddressBook); when(currentAddressBook.getNodeAccountIDPubKeyMap()).thenReturn(nodeAccountIDPubKeyMap); - when(commonDownloaderProperties.getConsensusRatio()).thenReturn(0.333); + when(commonDownloaderProperties.getConsensusRatio()).thenReturn(0.333f); } @Test @@ -160,7 +160,29 @@ void testCannotReachConsensus() { List fileStreamSignatures = Arrays.asList(buildBareBonesFileStreamSignature()); Exception e = assertThrows(SignatureVerificationException.class, () -> nodeSignatureVerifier .verify(fileStreamSignatures)); - assertTrue(e.getMessage().contains("Require at least 1/3 signature files to reach consensus")); + assertTrue(e.getMessage().contains("Requires at least 0.333 of signature files to reach consensus")); + } + + @Test + void testNoConsensusRequired() throws GeneralSecurityException { + Map nodeAccountIDPubKeyMap = new HashMap(); + nodeAccountIDPubKeyMap.put("0.0.3", publicKey); + nodeAccountIDPubKeyMap.put("0.0.4", publicKey); + nodeAccountIDPubKeyMap.put("0.0.5", publicKey); + nodeAccountIDPubKeyMap.put("0.0.6", publicKey); + nodeAccountIDPubKeyMap.put("0.0.7", publicKey); + nodeAccountIDPubKeyMap.put("0.0.8", publicKey); + nodeAccountIDPubKeyMap.put("0.0.9", publicKey); + nodeAccountIDPubKeyMap.put("0.0.10", publicKey); + + when(currentAddressBook.getNodeAccountIDPubKeyMap()).thenReturn(nodeAccountIDPubKeyMap); + when(commonDownloaderProperties.getConsensusRatio()).thenReturn(0f); + + byte[] fileHash = TestUtils.generateRandomByteArray(48); + byte[] fileHashSignature = signHash(fileHash); + + // only 1 node node necessary + nodeSignatureVerifier.verify(List.of()); } @Test @@ -189,6 +211,34 @@ void testVerifiedWithOneThirdConsensus() throws GeneralSecurityException { .verify(Arrays.asList(fileStreamSignatureNode3, fileStreamSignatureNode5)); } + @Test + void testVerifiedWithFullConsensusRequired() throws GeneralSecurityException { + Map nodeAccountIDPubKeyMap = new HashMap(); + nodeAccountIDPubKeyMap.put("0.0.3", publicKey); + nodeAccountIDPubKeyMap.put("0.0.4", publicKey); + nodeAccountIDPubKeyMap.put("0.0.5", publicKey); + when(currentAddressBook.getNodeAccountIDPubKeyMap()).thenReturn(nodeAccountIDPubKeyMap); + when(commonDownloaderProperties.getConsensusRatio()).thenReturn(1f); + + byte[] fileHash = TestUtils.generateRandomByteArray(48); + byte[] fileHashSignature = signHash(fileHash); + + FileStreamSignature fileStreamSignatureNode3 = buildFileStreamSignature(fileHash, fileHashSignature, + null, null); + fileStreamSignatureNode3.setNodeAccountId(new EntityId(0L, 0L, 3L, EntityTypeEnum.ACCOUNT.getId())); + + FileStreamSignature fileStreamSignatureNode4 = buildFileStreamSignature(fileHash, fileHashSignature, + null, null); + fileStreamSignatureNode4.setNodeAccountId(new EntityId(0L, 0L, 4L, EntityTypeEnum.ACCOUNT.getId())); + + FileStreamSignature fileStreamSignatureNode5 = buildFileStreamSignature(fileHash, fileHashSignature, + null, null); + fileStreamSignatureNode5.setNodeAccountId(new EntityId(0L, 0L, 5L, EntityTypeEnum.ACCOUNT.getId())); + + nodeSignatureVerifier + .verify(Arrays.asList(fileStreamSignatureNode3, fileStreamSignatureNode4, fileStreamSignatureNode5)); + } + @Test void testNoSignatureType() throws GeneralSecurityException { From 059eab93d6bf3c458d2155fe1cd8a061fff4f70b Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Thu, 13 May 2021 15:17:49 -0500 Subject: [PATCH 6/9] Collapse metrics and address some feedback Signed-off-by: Nana-EC --- docs/troubleshooting.md | 2 +- .../importer/domain/FileStreamSignature.java | 7 ++- .../downloader/NodeSignatureVerifier.java | 57 ++++++++++--------- .../downloader/NodeSignatureVerifierTest.java | 49 ++++++++++++++-- 4 files changed, 78 insertions(+), 37 deletions(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index dd63fe7e682..fcf4c186e2a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -249,7 +249,7 @@ Alerts: Low-Priority PagerDuty Alert during business hours only Response: Requir | `Missing signature for file` | LOW | | | `Error saving file in database` | NONE | HIGH (if 30 entries in 1 min) | | `Failed downloading` | NONE | HIGH (if 30 entries in 1 min) | -| `File could not be verified by at least 1/3 of nodes | NONE | HIGH (if 30 entries in 1 min) | +| `Insufficient downloaded signature file count, requires at least` | NONE | HIGH (if 30 entries in 1 min) | | `Signature verification failed` | NONE | HIGH (if 30 entries in 1 min) | | `Unable to connect to database` | NONE | HIGH (if 30 entries in 1 min) | | `Unable to fetch entity types` | NONE | HIGH (if 30 entries in 1 min) | diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java index 0970cd7f5c3..1a181c5ee6d 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java @@ -65,9 +65,10 @@ public String getNodeAccountIdString() { } public enum SignatureStatus { - DOWNLOADED, // Signature has been downloaded and parsed but not verified - VERIFIED, // Signature has been verified against the node's public key - CONSENSUS_REACHED // At least 1/3 of all nodes have been verified + DOWNLOADED, // Signature has been downloaded and parsed but not verified + VERIFIED, // Signature has been verified against the node's public key + CONSENSUS_REACHED, // At least 1/3 of all nodes have been verified + NOT_FOUND, // Signature for given node was not found for download } @Getter diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index 4deb923936a..d7daec32706 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -36,6 +36,7 @@ import java.util.stream.Collectors; import javax.inject.Named; import lombok.extern.log4j.Log4j2; +import org.springframework.util.CollectionUtils; import com.hedera.mirror.importer.addressbook.AddressBookService; import com.hedera.mirror.importer.domain.AddressBook; @@ -54,9 +55,7 @@ public class NodeSignatureVerifier { // Metrics private final MeterRegistry meterRegistry; - private final Map metricMap = new ConcurrentHashMap<>(); - private final Counter.Builder missingNodeSignatureFileMetric; - private final Counter.Builder signatureVerificationMetric; + private final Map nodeSignatureStatusMetricMap = new ConcurrentHashMap<>(); public NodeSignatureVerifier(AddressBookService addressBookService, CommonDownloaderProperties commonDownloaderProperties, @@ -101,8 +100,9 @@ public void verify(Collection signatures) throws SignatureV long nodeCount = nodeAccountIDPubKeyMap.size(); if (!canReachConsensus(sigFileCount, nodeCount)) { throw new SignatureVerificationException(String.format( - "Requires at least %s of signature files to reach consensus, got %d out of %d for file %s: %s", - String.format("%.03f", commonDownloaderProperties.getConsensusRatio()), + "Insufficient downloaded signature file count, requires at least %.03f to reach consensus, got %d" + + " out of %d for file %s: %s", + commonDownloaderProperties.getConsensusRatio(), sigFileCount, nodeCount, filename, @@ -132,7 +132,7 @@ public void verify(Collection signatures) throws SignatureV log.warn("Verified signature file {} reached consensus but with some errors: {}", filename, statusMap(signatures, nodeAccountIDPubKeyMap)); return; - } else if (commonDownloaderProperties.getConsensusRatio() == 0) { + } else if (commonDownloaderProperties.getConsensusRatio() == 0 && signatureHashMap.size() > 0) { log.debug("Signature file {} does not require consensus, skipping verification", filename); return; } @@ -194,35 +194,36 @@ private Map> statusMap(Collection seenNodes = new HashSet<>(); signatures.forEach(signature -> { seenNodes.add(signature.getNodeAccountId()); - EntityId nodeAccountId = signature.getNodeAccountId(); - - metricMap.computeIfAbsent(nodeAccountId, n -> signatureVerificationMetric - .tag("nodeAccount", nodeAccountId.getEntityNum().toString()) - .tag("realm", nodeAccountId.getRealmNum().toString()) - .tag("shard", nodeAccountId.getShardNum().toString()) - .tag("status", signature.getStatus().toString()) - .tag("type", signature.getStreamType().toString()) - .register(meterRegistry)) - .increment(); }); Set missingNodes = new TreeSet<>(Sets.difference( nodeAccountIDPubKeyMap.keySet().stream().map(x -> EntityId.of(x, EntityTypeEnum.ACCOUNT)) .collect(Collectors.toSet()), seenNodes)); - statusMap.put("MISSING", missingNodes); - statusMap.remove(SignatureStatus.CONSENSUS_REACHED.toString()); - - String streamType = signatures.stream().map(FileStreamSignature::getStreamType).findFirst().toString(); - missingNodes.forEach(nodeAccountId -> metricMap - .computeIfAbsent(nodeAccountId, n -> missingNodeSignatureFileMetric - .tag("nodeAccount", n.getEntityNum().toString()) - .tag("realm", nodeAccountId.getRealmNum().toString()) - .tag("shard", nodeAccountId.getShardNum().toString()) - .tag("type", streamType) - .register(meterRegistry)) - .increment()); + statusMap.put(SignatureStatus.NOT_FOUND.toString(), missingNodes); + + String streamType = CollectionUtils.isEmpty(signatures) ? "unknown" : + signatures.stream().map(FileStreamSignature::getStreamType).findFirst().toString(); + for (Map.Entry> entry : statusMap.entrySet()) { + entry.getValue().forEach(nodeAccountId -> { + Counter counter = nodeSignatureStatusMetricMap.computeIfAbsent( + nodeAccountId, + n -> newStatusMetric(nodeAccountId, streamType, entry.getKey())); + counter.increment(); + }); + } return statusMap; } + + private Counter newStatusMetric(EntityId entityId, String streamType, String status) { + return Counter.builder("hedera.mirror.download.signature.verification") + .description("The number of signatures verified from a particular node") + .tag("nodeAccount", entityId.getEntityNum().toString()) + .tag("realm", entityId.getRealmNum().toString()) + .tag("shard", entityId.getShardNum().toString()) + .tag("type", streamType) + .tag("status", status) + .register(meterRegistry); + } } diff --git a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java index a7bf39e97f6..92314891d6d 100644 --- a/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java +++ b/hedera-mirror-importer/src/test/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifierTest.java @@ -160,11 +160,11 @@ void testCannotReachConsensus() { List fileStreamSignatures = Arrays.asList(buildBareBonesFileStreamSignature()); Exception e = assertThrows(SignatureVerificationException.class, () -> nodeSignatureVerifier .verify(fileStreamSignatures)); - assertTrue(e.getMessage().contains("Requires at least 0.333 of signature files to reach consensus")); + assertTrue(e.getMessage().contains("Insufficient downloaded signature file count, requires at least 0.333")); } @Test - void testNoConsensusRequired() throws GeneralSecurityException { + void testNoConsensusRequiredWithVerifiedSignatureFiles() throws GeneralSecurityException { Map nodeAccountIDPubKeyMap = new HashMap(); nodeAccountIDPubKeyMap.put("0.0.3", publicKey); nodeAccountIDPubKeyMap.put("0.0.4", publicKey); @@ -181,8 +181,31 @@ void testNoConsensusRequired() throws GeneralSecurityException { byte[] fileHash = TestUtils.generateRandomByteArray(48); byte[] fileHashSignature = signHash(fileHash); + FileStreamSignature fileStreamSignatureNode = buildFileStreamSignature(fileHash, fileHashSignature, + null, null); + // only 1 node node necessary - nodeSignatureVerifier.verify(List.of()); + nodeSignatureVerifier.verify(List.of(fileStreamSignatureNode)); + } + + @Test + void testNoConsensusRequiredWithNoVerifiedSignatureFiles() throws GeneralSecurityException { + Map nodeAccountIDPubKeyMap = new HashMap(); + nodeAccountIDPubKeyMap.put("0.0.3", publicKey); + nodeAccountIDPubKeyMap.put("0.0.4", publicKey); + nodeAccountIDPubKeyMap.put("0.0.5", publicKey); + nodeAccountIDPubKeyMap.put("0.0.6", publicKey); + nodeAccountIDPubKeyMap.put("0.0.7", publicKey); + nodeAccountIDPubKeyMap.put("0.0.8", publicKey); + nodeAccountIDPubKeyMap.put("0.0.9", publicKey); + nodeAccountIDPubKeyMap.put("0.0.10", publicKey); + + when(currentAddressBook.getNodeAccountIDPubKeyMap()).thenReturn(nodeAccountIDPubKeyMap); + when(commonDownloaderProperties.getConsensusRatio()).thenReturn(0f); + + Exception e = assertThrows(SignatureVerificationException.class, () -> nodeSignatureVerifier + .verify(List.of())); + assertTrue(e.getMessage().contains("Signature verification failed for file")); } @Test @@ -196,6 +219,22 @@ void testVerifiedWithOneThirdConsensus() throws GeneralSecurityException { byte[] fileHash = TestUtils.generateRandomByteArray(48); byte[] fileHashSignature = signHash(fileHash); + nodeSignatureVerifier + .verify(Arrays.asList(buildFileStreamSignature(fileHash, fileHashSignature, + null, null))); + } + + @Test + void testVerifiedWithOneThirdConsensusWithMissingSignatures() throws GeneralSecurityException { + Map nodeAccountIDPubKeyMap = new HashMap(); + nodeAccountIDPubKeyMap.put("0.0.3", publicKey); + nodeAccountIDPubKeyMap.put("0.0.4", publicKey); + nodeAccountIDPubKeyMap.put("0.0.5", publicKey); + when(currentAddressBook.getNodeAccountIDPubKeyMap()).thenReturn(nodeAccountIDPubKeyMap); + + byte[] fileHash = TestUtils.generateRandomByteArray(48); + byte[] fileHashSignature = signHash(fileHash); + FileStreamSignature fileStreamSignatureNode3 = buildFileStreamSignature(fileHash, fileHashSignature, null, null); @@ -205,10 +244,10 @@ void testVerifiedWithOneThirdConsensus() throws GeneralSecurityException { fileStreamSignatureNode4.setNodeAccountId(new EntityId(0L, 0L, 4L, EntityTypeEnum.ACCOUNT.getId())); FileStreamSignature fileStreamSignatureNode5 = buildFileStreamSignature(fileHash, null, null, null); - fileStreamSignatureNode4.setNodeAccountId(new EntityId(0L, 0L, 5L, EntityTypeEnum.ACCOUNT.getId())); + fileStreamSignatureNode5.setNodeAccountId(new EntityId(0L, 0L, 5L, EntityTypeEnum.ACCOUNT.getId())); nodeSignatureVerifier - .verify(Arrays.asList(fileStreamSignatureNode3, fileStreamSignatureNode5)); + .verify(Arrays.asList(fileStreamSignatureNode3, fileStreamSignatureNode4, fileStreamSignatureNode5)); } @Test From cced0802c9f18469f03cc53ef2c459425d2cd6af Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Thu, 13 May 2021 16:29:18 -0500 Subject: [PATCH 7/9] Removed undefiend counters Signed-off-by: Nana-EC --- .../mirror/importer/downloader/NodeSignatureVerifier.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index d7daec32706..4e7da4306db 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -63,11 +63,6 @@ public NodeSignatureVerifier(AddressBookService addressBookService, this.addressBookService = addressBookService; this.commonDownloaderProperties = commonDownloaderProperties; this.meterRegistry = meterRegistry; - - missingNodeSignatureFileMetric = Counter.builder("hedera.mirror.download.signature.missing") - .description("The number of nodes whose signatures are missing from the consensus process"); - signatureVerificationMetric = Counter.builder("hedera.mirror.download.signature.verification") - .description("The number of signatures verified from a particular node"); } private boolean canReachConsensus(long actualNodes, long expectedNodes) { From bb652c335266fc3a8936754eddc6270918ecc254 Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Thu, 13 May 2021 18:05:40 -0500 Subject: [PATCH 8/9] Moved consesusratio 0 check and updated doc Signed-off-by: Nana-EC --- docs/troubleshooting.md | 2 +- .../importer/downloader/NodeSignatureVerifier.java | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index fcf4c186e2a..aff7ab9e8cc 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -99,7 +99,7 @@ Following is list of error messages and how to begin handling issues when they a - There is no immediate fix. Bring to team's attention immediately (during reasonable hours, otherwise next morning). -- `File could not be verified by at least 1/3 of nodes` +- `Insufficient downloaded signature file count, requires at least` This can happen if 1. Some mainnet nodes are still in the process of uploading their signatures for the latest file (benign case). diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java index 4e7da4306db..d96c0064c2d 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/downloader/NodeSignatureVerifier.java @@ -111,6 +111,11 @@ public void verify(Collection signatures) throws SignatureV } } + if (commonDownloaderProperties.getConsensusRatio() == 0 && signatureHashMap.size() > 0) { + log.debug("Signature file {} does not require consensus, skipping consensus check", filename); + return; + } + for (String key : signatureHashMap.keySet()) { Collection validatedSignatures = signatureHashMap.get(key); @@ -127,9 +132,6 @@ public void verify(Collection signatures) throws SignatureV log.warn("Verified signature file {} reached consensus but with some errors: {}", filename, statusMap(signatures, nodeAccountIDPubKeyMap)); return; - } else if (commonDownloaderProperties.getConsensusRatio() == 0 && signatureHashMap.size() > 0) { - log.debug("Signature file {} does not require consensus, skipping verification", filename); - return; } throw new SignatureVerificationException("Signature verification failed for file " + filename + ": " + statusMap(signatures, nodeAccountIDPubKeyMap)); @@ -187,9 +189,7 @@ private Map> statusMap(Collection seenNodes = new HashSet<>(); - signatures.forEach(signature -> { - seenNodes.add(signature.getNodeAccountId()); - }); + signatures.forEach(signature -> seenNodes.add(signature.getNodeAccountId())); Set missingNodes = new TreeSet<>(Sets.difference( nodeAccountIDPubKeyMap.keySet().stream().map(x -> EntityId.of(x, EntityTypeEnum.ACCOUNT)) From 6782926904520b30175317eb2a0da9b5c4b49c2f Mon Sep 17 00:00:00 2001 From: Nana-EC Date: Fri, 14 May 2021 13:09:08 -0500 Subject: [PATCH 9/9] Updated CONSENSUS_REACHED SignatureStatus comment Signed-off-by: Nana-EC --- .../com/hedera/mirror/importer/domain/FileStreamSignature.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java index 1a181c5ee6d..1ad95c7ebaa 100644 --- a/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java +++ b/hedera-mirror-importer/src/main/java/com/hedera/mirror/importer/domain/FileStreamSignature.java @@ -67,7 +67,7 @@ public String getNodeAccountIdString() { public enum SignatureStatus { DOWNLOADED, // Signature has been downloaded and parsed but not verified VERIFIED, // Signature has been verified against the node's public key - CONSENSUS_REACHED, // At least 1/3 of all nodes have been verified + CONSENSUS_REACHED, // Signature verification consensus reached by a node count greater than the consensusRatio NOT_FOUND, // Signature for given node was not found for download }