Skip to content

chore: Improvements to Block Footer and Proof Support #1891

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.

Sign up for GitHub

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

Draft
Nana-EC wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

chore: Improvements to Block Footer and Proof Support #1891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e8360d1
Increased test coverage for block items and expanded verification logic
Nana-EC Nov 29, 2025
a2fdd90
Apply spotless
Nana-EC Nov 29, 2025
f68023e
Fix tests
Nana-EC Nov 29, 2025
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
13 changes: 10 additions & 3 deletions ...estFixtures/java/org/hiero/block/node/app/fixtures/blocks/SimpleTestBlockItemBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import com.hedera.hapi.block.stream.BlockItem;
import com.hedera.hapi.block.stream.BlockItem.ItemOneOfType;
import com.hedera.hapi.block.stream.BlockProof;
import com.hedera.hapi.block.stream.ChainOfTrustProof;
import com.hedera.hapi.block.stream.MerkleSiblingHash;
import com.hedera.hapi.block.stream.TssSignedBlockProof;
import com.hedera.hapi.block.stream.input.EventHeader;
import com.hedera.hapi.block.stream.input.RoundHeader;
Expand Down Expand Up @@ -74,12 +76,17 @@ public static BlockProof createBlockProof(final long blockNumber) {
TssSignedBlockProof.Builder tssBuildder =
TssSignedBlockProof.newBuilder().blockSignature(Bytes.wrap("block_signature".getBytes()));

BlockProof blockProof = BlockProof.newBuilder()
return BlockProof.newBuilder()
.block(blockNumber)
.siblingHashes(List.of(MerkleSiblingHash.newBuilder()
.siblingHash(Bytes.wrap("sibling_hash".getBytes()))
.build()))
.signedBlockProof(tssBuildder)
.verificationKey(Bytes.wrap("verification_key".getBytes()))
.verificationKeyProof(ChainOfTrustProof.newBuilder()
.wrapsProof(Bytes.wrap("verificationKeyProof".getBytes()))
.build())
.build();

return blockProof;
}

public static Bytes createBlockProofUnparsed(final long blockNumber) {
Expand Down
8 changes: 8 additions & 0 deletions ...k-node/spi/src/test/java/org/hiero/block/node/spi/historicalblocks/BlockAccessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import com.hedera.hapi.block.stream.BlockItem;
import com.hedera.hapi.block.stream.BlockItem.ItemOneOfType;
import com.hedera.hapi.block.stream.BlockProof;
import com.hedera.hapi.block.stream.ChainOfTrustProof;
import com.hedera.hapi.block.stream.MerkleSiblingHash;
import com.hedera.hapi.block.stream.TssSignedBlockProof;
import com.hedera.hapi.block.stream.input.RoundHeader;
import com.hedera.hapi.block.stream.output.BlockHeader;
Expand Down Expand Up @@ -43,9 +45,15 @@ public class BlockAccessorTest {
ItemOneOfType.BLOCK_PROOF,
BlockProof.newBuilder()
.block(0)
.siblingHashes(MerkleSiblingHash.newBuilder()
.siblingHash(Bytes.wrap("sibling_hash".getBytes()))
.build())
.signedBlockProof(TssSignedBlockProof.newBuilder()
.blockSignature(Bytes.wrap("signature"))
.build())
.verificationKey(Bytes.wrap("verification_key".getBytes()))
.verificationKeyProof(ChainOfTrustProof.newBuilder()
.wrapsProof(Bytes.wrap("verificationKeyProof".getBytes())))
.build()))));
private static final Bytes SAMPLE_BLOCK_PROTOBUF_BYTES = Block.PROTOBUF.toBytes(SAMPLE_BLOCK);
private static final Bytes SAMPLE_BLOCK_ZSTD_PROTOBUF_BYTES =
Expand Down
45 changes: 35 additions & 10 deletions ...c/main/java/org/hiero/block/node/verification/session/impl/ExtendedMerkleTreeSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import static org.hiero.block.common.hasher.HashingUtilities.noThrowSha384HashOf;

import com.hedera.hapi.block.stream.BlockProof;
import com.hedera.hapi.block.stream.SignedRecordFileProof;
import com.hedera.hapi.block.stream.StateProof;
import com.hedera.hapi.block.stream.TssSignedBlockProof;
import com.hedera.hapi.block.stream.output.BlockFooter;
import com.hedera.hapi.block.stream.output.BlockHeader;
import com.hedera.pbj.runtime.ParseException;
Expand Down Expand Up @@ -55,8 +58,11 @@
private BlockHeader blockHeader = null;

private BlockFooter blockFooter = null;
private TssSignedBlockProof tssSignedBlockProof;
private StateProof blockStateProof;

Check warning on line 62 in block-node/verification/src/main/java/org/hiero/block/node/verification/session/impl/ExtendedMerkleTreeSession.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

block-node/verification/src/main/java/org/hiero/block/node/verification/session/impl/ExtendedMerkleTreeSession.java#L62 

Avoid unused private fields such as 'blockStateProof'.
private SignedRecordFileProof signedRecordFileProof;

Check warning on line 63 in block-node/verification/src/main/java/org/hiero/block/node/verification/session/impl/ExtendedMerkleTreeSession.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

block-node/verification/src/main/java/org/hiero/block/node/verification/session/impl/ExtendedMerkleTreeSession.java#L63 

Avoid unused private fields such as 'signedRecordFileProof'.

private List<BlockProof> blockProofs = new ArrayList<>();
private int blockProofsReceived = 0;
Comment on lines +61 to +65
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

This probably won't work. We can have multiple of the same type of block proof.
StateProof type block proofs, in particular, are very likely to have multiple on a given block.
TSS signed proofs are currently expected to be singular, but that may not always be true. Signed record file proof type will probably always be singular, but we perhaps shouldn't assume that (for symmetry, if nothing else).


public ExtendedMerkleTreeSession(final long blockNumber, final BlockSource blockSource) {
this.blockNumber = blockNumber;
Expand Down Expand Up @@ -94,16 +100,31 @@
// append block proofs
case BLOCK_PROOF -> {
BlockProof blockProof = BlockProof.PROTOBUF.parse(item.blockProof());
blockProofs.add(blockProof);
switch (blockProof.proof().kind()) {
case BLOCK_STATE_PROOF -> this.blockStateProof = blockProof.blockStateProof();
case SIGNED_BLOCK_PROOF -> this.tssSignedBlockProof = blockProof.signedBlockProof();
case SIGNED_RECORD_FILE_PROOF ->
this.signedRecordFileProof = blockProof.signedRecordFileProof();
default -> {
continue;
}
}

blockProofsReceived++;
Comment on lines +103 to +113
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

This appears to be assuming only one of any given type of proof. That is not what we expect; there may well be multiples of the same type. The original (add all of them to a set and process all as a set) seems more correct.

Copy link
Contributor Author

@Nana-EC Nana-EC Dec 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Indeed that was the assumption, good note, will re-approach this

}
case REDACTED_ITEM -> LOGGER.log(WARNING, "Redacted item observed in block {0}", blockNumber);
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

This is tricky.
A live block cannot have redaction.
An historical block may.
In the former case the block must be rejected (bad block).
In the latter case, we don't need to log it, we need to include that redacted hash in the correct subtree and proceed.

Copy link
Contributor Author

@Nana-EC Nana-EC Dec 2, 2025

Choose a reason for hiding this comment

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

Yes tricky indeed, hmm, VerificationSession would have to be aware of the source of the Block.
If it's from a CN publisher then reject otherwise include hash as is without rehashing.
The warning log is to highlight we have things to do in the code.
Might be worth dropping this part and just capturing a ticket to come back to

Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 2, 2025

Choose a reason for hiding this comment

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

Probably better to have an issue filed to revisit.
Generally, because WARNING logs are interpreted as software bugs by some testing processes, INFO would be the highest priority for a "we need to come back and improve this" log.

case FILTERED_ITEM_HASH -> LOGGER.log(WARNING, "Filtered item hash observed in block {0}", blockNumber);
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

This one is fun.

  1. From live, this is strictly prohibited
  2. From storage or another block node in tier 1 this is strictly prohibited.
  3. From any source in tier 2 this is permitted.

In all cases filtered items still need to be added to the proper subtree as a pre-computed hash (which I do not believe the current hasher supports, future work to handle that).

This suggests that we probably need two different verification plugins. One that is "strict" and runs in tier 1, and another that is "lenient" and may run in tier 2 if an operator chooses. (yes, we could do this with configuration, but I believe we'd be better off and much safer to just have two different plugins).

case RECORD_FILE -> LOGGER.log(WARNING, "Record file observed in block {0}", blockNumber);
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

We actually need to parse and process these; for now we probably want an info log; not warning.

case UNSET -> LOGGER.log(WARNING, "Unset block item observed in block {1}", kind, blockNumber);
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

This shouldn't happen.
It also probably shouldn't be a warning log; I would expect an info log and immediate failure of the verification (because there is a completely unprocessable block item).

Copy link
Contributor Author

@Nana-EC Nana-EC Dec 2, 2025

Choose a reason for hiding this comment

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

I believe a new blockitem type we are unaware of would show up as UNSET.
Need to confirm with tests

default -> LOGGER.log(WARNING, "Unknown block item type {0} in block {1}", kind, blockNumber);
Copy link
Contributor

@jsync-swirlds jsync-swirlds Dec 1, 2025

Choose a reason for hiding this comment

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

We shouldn't have default; the enumerated value is comprehensive and we should ensure every known enum is handled (which the compiler will only do if there is no default).
Any enumerated type we don't yet handle should be in a different place (the unknown values map); not mixed in with existing block items.

}
}

// since we are only expecting 1 block proof per block, we can finalize verification here
// however in the future, we might want to revisit this if we expect multiple proofs
// and use the EndOfBlock signal to finalize verification.
if (blockFooter != null && blockProofs.size() > 0) {
return finalizeVerification(blockProofs.get(0));
if (blockFooter != null && blockProofsReceived > 0) {
return finalizeVerification();
}

// was not able to finalize verification yet
Expand All @@ -114,10 +135,9 @@
* Finalizes the block verification by computing the final block hash,
* verifying its signature, and updating metrics accordingly.
*
* @param blockProof the block proof
* @return VerificationNotification indicating the result of the verification
*/
protected VerificationNotification finalizeVerification(BlockProof blockProof) {
protected VerificationNotification finalizeVerification() {

// pre-checks
if (previousBlockHash != Bytes.EMPTY
Expand All @@ -139,12 +159,17 @@
traceDataHasher,
previousBlockHash);

final boolean verified =
verifySignature(blockRootHash, blockProof.signedBlockProof().blockSignature());

// set the previous block hash for the next block
previousBlockHash = blockRootHash;

// confirm block proofs (tssSignedBlockProof, blockStateProof or signedRecordFileProof), one must be valid.
// As of CN v0.68 only TSS proof is supported.
int validProofCount = 0;
if (tssSignedBlockProof != null && verifyTssSignature(blockRootHash, tssSignedBlockProof.blockSignature())) {
validProofCount++;
}

boolean verified = validProofCount > 0;
return new VerificationNotification(
verified, blockNumber, blockRootHash, verified ? new BlockUnparsed(blockItems) : null, blockSource);
}
Expand All @@ -156,7 +181,7 @@
* @param signature the signature to verify
* @return true if the signature is valid, false otherwise
*/
protected Boolean verifySignature(@NonNull Bytes hash, @NonNull Bytes signature) {
protected Boolean verifyTssSignature(@NonNull Bytes hash, @NonNull Bytes signature) {
// TODO we are close to having real TSS signature verification, we maybe should have a config if we are work on
// TODO preview or production block stream and hence which verification to use

Expand Down
66 changes: 58 additions & 8 deletions ...cation/src/test/java/org/hiero/block/node/verification/VerificationServicePluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,15 @@
import com.hedera.pbj.runtime.OneOf;
import com.hedera.pbj.runtime.ParseException;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.LinkedBlockingQueue;
import org.hiero.block.internal.BlockItemUnparsed;
import org.hiero.block.internal.BlockItemUnparsed.ItemOneOfType;
import org.hiero.block.node.app.fixtures.async.BlockingExecutor;
import org.hiero.block.node.app.fixtures.blocks.BlockUtils;
import org.hiero.block.node.app.fixtures.blocks.BlockUtils.SAMPLE_BLOCKS;
import org.hiero.block.node.app.fixtures.plugintest.NoBlocksHistoricalBlockFacility;
import org.hiero.block.node.app.fixtures.plugintest.PluginTestBase;
import org.hiero.block.node.app.fixtures.plugintest.TestHealthFacility;
Expand All @@ -36,7 +39,10 @@

public VerificationServicePluginTest() {
super(new BlockingExecutor(new LinkedBlockingQueue<>()));
start(new VerificationServicePlugin(), new NoBlocksHistoricalBlockFacility());
start(
new VerificationServicePlugin(),
new NoBlocksHistoricalBlockFacility(),
Map.of("PROMETHEUS_ENDPOINT_ENABLED", "false"));
}

@Test
Expand Down Expand Up @@ -70,14 +76,25 @@
}

@Test
@DisplayName("Test failed verification due to hash mismatch")
void testFailedVerification() throws IOException, ParseException {

BlockUtils.SampleBlockInfo sampleBlockInfo =
BlockUtils.getSampleBlockInfo(BlockUtils.SAMPLE_BLOCKS.HAPI_0_68_0_BLOCK_14);

List<BlockItemUnparsed> blockItems = sampleBlockInfo.blockUnparsed().blockItems();
// remove one block item, so the hash is no longer valid
blockItems.remove(3);
// create a list of block items with missing items to simulate a non-valid hash
List<BlockItemUnparsed> blockItems = new ArrayList<>();
blockItems.addAll(sampleBlockInfo
.blockUnparsed()
.blockItems()
.subList(0, sampleBlockInfo.blockUnparsed().blockItems().size() / 2));
blockItems.addAll(sampleBlockInfo
.blockUnparsed()
.blockItems()
.subList(
sampleBlockInfo.blockUnparsed().blockItems().size() / 2 + 2,
sampleBlockInfo.blockUnparsed().blockItems().size()));

long blockNumber = sampleBlockInfo.blockNumber();

blockMessaging.sendBlockItems(new BlockItems(blockItems, blockNumber));
Expand All @@ -101,14 +118,18 @@

@Test
@DisplayName("Test handleBlockItemsReceived without a block header")
void testHandleBlockItemsReceived_NoCurrentSession() throws IOException, ParseException {
void testHandleBlockItemsReceived_NoHeader() throws IOException, ParseException {

Check notice on line 121 in block-node/verification/src/test/java/org/hiero/block/node/verification/VerificationServicePluginTest.java

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

block-node/verification/src/test/java/org/hiero/block/node/verification/VerificationServicePluginTest.java#L121 

The JUnit 5 test method name 'testHandleBlockItemsReceived_NoHeader' doesn't match '[a-z][a-zA-Z0-9]*'
// create sample block data
BlockUtils.SampleBlockInfo sampleBlockInfo =
BlockUtils.getSampleBlockInfo(BlockUtils.SAMPLE_BLOCKS.HAPI_0_68_0_BLOCK_14);
long blockNumber = sampleBlockInfo.blockNumber();
List<BlockItemUnparsed> blockItems = sampleBlockInfo.blockUnparsed().blockItems();
// remove the header to simulate a case where receive items and have never received a header
blockItems.removeFirst();

// create a list of block items with no header
List<BlockItemUnparsed> blockItems = sampleBlockInfo
.blockUnparsed()
.blockItems()
.subList(1, sampleBlockInfo.blockUnparsed().blockItems().size());

// send some items to the plugin, they should be ignored
plugin.handleBlockItemsReceived(new BlockItems(blockItems, blockNumber));
// check we did not receive a block verification
Expand Down Expand Up @@ -205,4 +226,33 @@
assertNotNull(blockNotification);
assertFalse(blockNotification.success(), "The verification should be unsuccessful");
}

@Test
void testVerificationPluginPre068() throws IOException, ParseException {

BlockUtils.SampleBlockInfo sampleBlockInfo = BlockUtils.getSampleBlockInfo(SAMPLE_BLOCKS.HAPI_0_66_0_BLOCK_10);

List<BlockItemUnparsed> blockItems = sampleBlockInfo.blockUnparsed().blockItems();
long blockNumber = sampleBlockInfo.blockNumber();

blockMessaging.sendBlockItems(new BlockItems(blockItems, blockNumber));

// check we received a block verification
VerificationNotification blockNotification =
blockMessaging.getSentVerificationNotifications().getFirst();
assertNotNull(blockNotification);
assertEquals(
blockNumber,
blockNotification.blockNumber(),
"The block number should be the same as the one in the block header");
assertTrue(blockNotification.success(), "The verification should be successful");
assertEquals(
sampleBlockInfo.blockRootHash(),
blockNotification.blockHash(),
"The block hash should be the same as the one in the block header");
assertEquals(
sampleBlockInfo.blockUnparsed(),
blockNotification.block(),
"The block should be the same as the one sent");
}
}
2 changes: 1 addition & 1 deletion settings.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import org.gradlex.javamodule.moduleinfo.ExtraJavaModuleInfoPluginExtension

plugins {
id("org.hiero.gradle.build") version "0.6.3"
id("com.hedera.pbj.pbj-compiler") version "0.12.5" apply false
id("com.hedera.pbj.pbj-compiler") version "0.12.9" apply false
}

val hieroGroup = "org.hiero.block"
Expand Down
Loading
Loading