Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Block Verification Feature #414

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0ba868b
First working iteration, just make the hash valid :)
AlfredoG87 Dec 13, 2024
2d62e25
initial metrics dashboard and observations
AlfredoG87 Dec 13, 2024
53d0ebb
some simplifications and moving around code, also more logs for tests…
AlfredoG87 Dec 14, 2024
d86b0eb
Some more improvements and moving things to async model. however I th…
AlfredoG87 Dec 16, 2024
9b55f2b
clean-up halfway.
AlfredoG87 Dec 17, 2024
7f8e46e
major refactor, clean-up 80%
AlfredoG87 Dec 17, 2024
25d4bb0
added NoOp implementation for the VerificationService
AlfredoG87 Dec 17, 2024
9fe7505
added javadoc to BlockVerificationService interface
AlfredoG87 Dec 17, 2024
f2df2c5
some more javadocs
AlfredoG87 Dec 17, 2024
a6a7ddf
adding some extra exception handling.
AlfredoG87 Dec 17, 2024
f6ca146
cleanup
AlfredoG87 Dec 17, 2024
e104327
cleanup
AlfredoG87 Dec 17, 2024
61f2294
Fixing Existing Unit tests and removing an addition (requires org.che…
AlfredoG87 Dec 17, 2024
c7483f2
Unit test for StreamingTreeHasher and BlockVerificationService and so…
AlfredoG87 Dec 17, 2024
ab20ed6
VerificationInjectionModuleTest Unit test and spotless improvements
AlfredoG87 Dec 17, 2024
aeb8850
More unit tests
AlfredoG87 Dec 17, 2024
773425d
More unit tests, for BlockVerificationSessionSync
AlfredoG87 Dec 18, 2024
47efaee
More unit tests, for BlockVerificationSessionSync
AlfredoG87 Dec 18, 2024
95434d8
Adding UT Coverage for BlockVerificationSession related classes.
AlfredoG87 Dec 18, 2024
8c363e8
restoring original simulator properties file
AlfredoG87 Dec 18, 2024
606e416
improvements on performance for ASYNC
AlfredoG87 Dec 18, 2024
7c3d716
improvements for the dashboard and the config class so it logs the ac…
AlfredoG87 Dec 18, 2024
d209718
Added env mappings for new verification config properties
AlfredoG87 Dec 18, 2024
07f85d6
Added some missing javadocs
AlfredoG87 Dec 18, 2024
bd9fb75
adding env config properties documentation
AlfredoG87 Dec 18, 2024
863707c
Added remaining javadocs across the whole project
AlfredoG87 Dec 18, 2024
5856d2b
style fixes
AlfredoG87 Dec 18, 2024
36fcd2d
convert sha384HashTag to final
AlfredoG87 Dec 18, 2024
553985a
some extra tests and dashboard improvement, also update the dashboard…
AlfredoG87 Dec 18, 2024
eeb977b
remove extension
AlfredoG87 Dec 18, 2024
f173ed8
style fixes
AlfredoG87 Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4,433 changes: 2,699 additions & 1,734 deletions charts/hedera-block-node/dashboards/block-node-server.json

Large diffs are not rendered by default.

4,244 changes: 2,498 additions & 1,746 deletions server/docker/metrics/dashboards/block-node-server.json

Large diffs are not rendered by default.

30 changes: 17 additions & 13 deletions server/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@ The default configuration allows users to quickly get up and running without hav
ease of use at the trade-off of some insecure default configuration. Most configuration settings have appropriate
defaults and can be left unchanged. It is recommended to browse the properties below and adjust to your needs.

| Environment Variable | Description | Default Value |
|:---|:---|---:|
| PERSISTENCE_STORAGE_LIVE_ROOT_PATH | The root path for the live storage. | |
| PERSISTENCE_STORAGE_ARCHIVE_ROOT_PATH | The root path for the archive storage. | |
| PERSISTENCE_STORAGE_TYPE | Type of the persistence storage | BLOCK_AS_LOCAL_FILE |
| PERSISTENCE_STORAGE_COMPRESSION | Compression algorithm used during persistence (could be none as well) | ZSTD |
| PERSISTENCE_STORAGE_COMPRESSION_LEVEL | Compression level to be used by the compression algorithm | 3 |
| CONSUMER_TIMEOUT_THRESHOLD_MILLIS | Time to wait for subscribers before disconnecting in milliseconds | 1500 |
| SERVICE_DELAY_MILLIS | Service shutdown delay in milliseconds | 500 |
| MEDIATOR_RING_BUFFER_SIZE | Size of the ring buffer used by the mediator (must be a power of 2) | 67108864 |
| NOTIFIER_RING_BUFFER_SIZE | Size of the ring buffer used by the notifier (must be a power of 2) | 2048 |
| SERVER_PORT | The port the server will listen on | 8080 |
| SERVER_MAX_MESSAGE_SIZE_BYTES | The maximum size of a message frame in bytes | 1048576 |
| Environment Variable | Description | Default Value |
|:---------------------------------------|:-------------------------------------------------------------------------|--------------------:|
| PERSISTENCE_STORAGE_LIVE_ROOT_PATH | The root path for the live storage. | |
| PERSISTENCE_STORAGE_ARCHIVE_ROOT_PATH | The root path for the archive storage. | |
Comment on lines +12 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's stick with the old formatting of the md tables for now. I will make a discussion so that the team will be able to vote on what the formatting should be for the tables so we can all be on the same page.

| PERSISTENCE_STORAGE_TYPE | Type of the persistence storage | BLOCK_AS_LOCAL_FILE |
| PERSISTENCE_STORAGE_COMPRESSION | Compression algorithm used during persistence (could be none as well) | ZSTD |
| PERSISTENCE_STORAGE_COMPRESSION_LEVEL | Compression level to be used by the compression algorithm | 3 |
| CONSUMER_TIMEOUT_THRESHOLD_MILLIS | Time to wait for subscribers before disconnecting in milliseconds | 1500 |
| SERVICE_DELAY_MILLIS | Service shutdown delay in milliseconds | 500 |
| MEDIATOR_RING_BUFFER_SIZE | Size of the ring buffer used by the mediator (must be a power of 2) | 67108864 |
| NOTIFIER_RING_BUFFER_SIZE | Size of the ring buffer used by the notifier (must be a power of 2) | 2048 |
| SERVER_PORT | The port the server will listen on | 8080 |
| SERVER_MAX_MESSAGE_SIZE_BYTES | The maximum size of a message frame in bytes | 1048576 |
| VERIFICATION_ENABLED | Enables or disables the block verification process | true |
| VERIFICATION_SESSION_TYPE | The type of BlockVerificationSession to use, either `ASYNC` or `SYNC` | ASYNC |
| VERIFICATION_HASH_COMBINE_BATCH_SIZE | The number of hashes to combine into a single hash during verification | 32 |

Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.hedera.block.server.pbj.PbjInjectionModule;
import com.hedera.block.server.persistence.PersistenceInjectionModule;
import com.hedera.block.server.service.ServiceInjectionModule;
import com.hedera.block.server.verification.VerificationInjectionModule;
import com.swirlds.config.api.Configuration;
import dagger.BindsInstance;
import dagger.Component;
Expand All @@ -42,6 +43,7 @@
ConfigInjectionModule.class,
MetricsInjectionModule.class,
PbjInjectionModule.class,
VerificationInjectionModule.class,
})
public interface BlockNodeAppInjectionComponent {
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.hedera.block.server.persistence.storage.PersistenceStorageConfig;
import com.hedera.block.server.producer.ProducerConfig;
import com.hedera.block.server.service.ServiceConfig;
import com.hedera.block.server.verification.VerificationConfig;
import com.swirlds.common.metrics.config.MetricsConfig;
import com.swirlds.common.metrics.platform.prometheus.PrometheusConfig;
import com.swirlds.config.api.ConfigurationExtension;
Expand Down Expand Up @@ -56,6 +57,7 @@ public Set<Class<? extends Record>> getConfigDataTypes() {
ProducerConfig.class,
ConsumerConfig.class,
PersistenceStorageConfig.class,
ServerConfig.class);
ServerConfig.class,
VerificationConfig.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.hedera.block.server.notifier.NotifierConfig;
import com.hedera.block.server.persistence.storage.PersistenceStorageConfig;
import com.hedera.block.server.producer.ProducerConfig;
import com.hedera.block.server.verification.VerificationConfig;
import com.swirlds.common.metrics.config.MetricsConfig;
import com.swirlds.common.metrics.platform.prometheus.PrometheusConfig;
import com.swirlds.config.api.Configuration;
Expand Down Expand Up @@ -131,4 +132,16 @@ static ProducerConfig provideProducerConfig(Configuration configuration) {
static ServerConfig provideServerConfig(Configuration configuration) {
return configuration.getConfigData(ServerConfig.class);
}

/**
* Provides a verification configuration singleton using the configuration.
*
* @param configuration is the configuration singleton
* @return a verification configuration singleton
*/
@Singleton
@Provides
static VerificationConfig provideVerificationConfig(Configuration configuration) {
return configuration.getConfigData(VerificationConfig.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ public final class ServerMappedConfigSourceInitializer {
new ConfigMapping("server.maxMessageSizeBytes", "SERVER_MAX_MESSAGE_SIZE_BYTES"),
new ConfigMapping("server.port", "SERVER_PORT"),
new ConfigMapping("prometheus.endpointEnabled", "PROMETHEUS_ENDPOINT_ENABLED"),
new ConfigMapping("prometheus.endpointPortNumber", "PROMETHEUS_ENDPOINT_PORT_NUMBER"));
new ConfigMapping("prometheus.endpointPortNumber", "PROMETHEUS_ENDPOINT_PORT_NUMBER"),
new ConfigMapping("verification.enabled", "VERIFICATION_ENABLED"),
new ConfigMapping("verification.sessionType", "VERIFICATION_SESSION_TYPE"),
new ConfigMapping("verification.hashCombineBatchSize", "VERIFICATION_HASH_COMBINE_BATCH_SIZE"));

private ServerMappedConfigSourceInitializer() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,23 @@ public enum Counter implements MetricMetadata {
/** The number of single blocks not found via the singleBlock rpc service. */
SingleBlocksNotFound("single_blocks_not_found", "Single Blocks Not Found"),

// Verification counters

/** The number of blocks received for verification. */
VerificationBlocksReceived("verification_blocks_received", "Blocks Received for Verification"),

/** The number of blocks verified successfully. */
VerificationBlocksVerified("verification_blocks_verified", "Blocks Verified"),

/** The number of blocks that failed verification. */
VerificationBlocksFailed("verification_blocks_failed", "Blocks Failed Verification"),

/** The number of blocks that failed verification due to an error. */
VerificationBlocksError("verification_blocks_error", "Blocks Verification Error"),

/** The time in nanoseconds taken to verify a block */
VerificationBlockTime("verification_block_time", "Block Verification Time"),

// Error counters

/** The number of errors encountered by the live block stream mediator. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ public void publish(@NonNull List<BlockItemUnparsed> blockItems) {
}
}

/**
* Builds an error stream response.
*
* @return the error stream response
*/
@NonNull
static PublishStreamResponse buildErrorStreamResponse() {
// TODO: Replace this with a real error enum.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ public Pipeline<? super Bytes> open(
}
}

/**
* Executes the unary singleBlock gRPC method.
*
* @param singleBlockRequest the single block request
* @return the single block response
*/
SingleBlockResponseUnparsed singleBlock(SingleBlockRequest singleBlockRequest) {

LOGGER.log(DEBUG, "Executing Unary singleBlock gRPC method");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.hedera.block.server.producer.ProducerBlockItemObserver;
import com.hedera.block.server.producer.ProducerConfig;
import com.hedera.block.server.service.ServiceStatus;
import com.hedera.block.server.verification.StreamVerificationHandlerImpl;
import com.hedera.hapi.block.BlockItemUnparsed;
import com.hedera.hapi.block.PublishStreamRequestUnparsed;
import com.hedera.hapi.block.PublishStreamResponse;
Expand Down Expand Up @@ -65,6 +66,7 @@ public class PbjBlockStreamServiceProxy implements PbjBlockStreamService {
* @param streamMediator the live stream mediator
* @param serviceStatus the service status
* @param streamPersistenceHandler the stream persistence handler
* @param streamVerificationHandler the stream verification handler
* @param notifier the notifier
* @param blockNodeContext the block node context
*/
Expand All @@ -73,13 +75,15 @@ public PbjBlockStreamServiceProxy(
@NonNull final LiveStreamMediator streamMediator,
@NonNull final ServiceStatus serviceStatus,
@NonNull final BlockNodeEventHandler<ObjectEvent<SubscribeStreamResponseUnparsed>> streamPersistenceHandler,
@NonNull final StreamVerificationHandlerImpl streamVerificationHandler,
@NonNull final Notifier notifier,
@NonNull final BlockNodeContext blockNodeContext) {
this.serviceStatus = serviceStatus;
this.notifier = notifier;
this.blockNodeContext = blockNodeContext;

streamMediator.subscribe(streamPersistenceHandler);
streamMediator.subscribe(streamVerificationHandler);
this.streamMediator = streamMediator;
Comment on lines 81 to 87
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add null checks here on all lines 81-87 as they are assignment and will be used later, lets fail early here.

}

Expand Down Expand Up @@ -119,6 +123,12 @@ public Pipeline<? super Bytes> open(
}
}

/**
* Publishes the block stream.
*
* @param helidonProducerObserver the helidon producer observer
* @return the pipeline
*/
Pipeline<List<BlockItemUnparsed>> publishBlockStream(
Pipeline<? super PublishStreamResponse> helidonProducerObserver) {
LOGGER.log(DEBUG, "Executing bidirectional publishBlockStream gRPC method");
Expand Down Expand Up @@ -153,6 +163,12 @@ Pipeline<List<BlockItemUnparsed>> publishBlockStream(
}
}

/**
* Subscribes to the block stream.
*
* @param subscribeStreamRequest the subscribe stream request
* @param subscribeStreamResponseObserver the subscribe stream response observer
*/
void subscribeBlockStream(
SubscribeStreamRequest subscribeStreamRequest,
Pipeline<? super SubscribeStreamResponseUnparsed> subscribeStreamResponseObserver) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,25 @@ public enum CompressionType {
private final int minCompressionLevel;
private final int maxCompressionLevel;

/**
* Constructs a new instance of {@link CompressionType}.
*
* @param minCompressionLevel the minimum compression level
* @param maxCompressionLevel the maximum compression level
*/
CompressionType(final int minCompressionLevel, final int maxCompressionLevel) {
this.minCompressionLevel = minCompressionLevel;
this.maxCompressionLevel = maxCompressionLevel;
}

/**
* This method verifies that the compression level is within the
* acceptable range for the given compression type.
*
* @param levelToCheck the compression level to check
* @throws IllegalArgumentException if the compression level is not within
* the acceptable range
*/
Comment on lines +176 to +194
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the docs! I've missed that :)

public void verifyCompressionLevel(final int levelToCheck) {
Preconditions.requireInRange(levelToCheck, minCompressionLevel, maxCompressionLevel);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,7 @@
/**
* A marker interface that groups all writers that operate on a local file
* system.
*
* @param <V> the type of the value to be written
Comment on lines +22 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the docs again :)

*/
interface LocalBlockWriter<V> extends BlockWriter<V> {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright (C) 2024 Hedera Hashgraph, LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.hedera.block.server.verification;

/**
* An enum representing the status of block verification.
*/
public enum BlockVerificationStatus {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also add an UNVERIFIED option which would mean that this block is not yet verified? Or does the core logic and design do not support that?

/**
* The Block has been verified.
*/
VERIFIED,
/**
* The Block failed verification, either due to an invalid signature or an invalid hash.
*/
SIGNATURE_INVALID
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implies that it's strictly signature issue, maybe add invalid hash enum or change name to something more ambiguous, as we are not sure at this point what is the root cause ?

}
Loading
Loading