Hash sig #34

kamilsa · 2025-11-21T04:21:39Z

Update up to the leanSpec 49f70c7:

Introduce hash-sig
Refactorings and documentations according to spec

remove num_validators, update genesis_time, and enhance genesis state generation

…tation

…k methods in ValidatorKeysManifestMock

Signed-off-by: turuslan <[email protected]>

Copilot

Pull request overview

This PR introduces XMSS (eXtended Merkle Signature Scheme) hash-based signatures to replace placeholder signatures in the consensus protocol. The implementation integrates the qdrvm-crates dependency for cryptographic operations and refactors the validator key management system.

Key Changes:

Adds XMSS signature support through new crypto::xmss module with provider interface, types, and utility functions
Removes num_validators from Config in favor of dynamic validator list stored in State
Implements ValidatorKeysManifest for loading and managing validator public keys from YAML manifests
Refactors signature verification throughout the fork choice and state transition systems

Reviewed changes

Copilot reviewed 57 out of 60 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
vcpkg.json	Adds qdrvm-crates dependency for XMSS cryptography
vcpkg-overlay/qdrvm-crates/*	Configures qdrvm-crates package with c_hash_sig binding
src/crypto/xmss/*	Implements XMSS provider, types, and key loading utilities
src/app/validator_keys_manifest	Adds validator key manifest loading and management
src/types/{config,state,signature,validator}.hpp	Refactors types to use XMSS keys and dynamic validator lists
src/blockchain/state_transition_function.*	Updates genesis state generation to populate validators from manifest
src/blockchain/fork_choice.*	Implements XMSS signature verification for blocks and attestations
tests/unit/crypto/*	Adds XMSS provider tests
tests/unit/blockchain/*	Updates tests for new validator registry and key manifest interfaces
tests/mock/*	Adds mocks for XMSS provider and validator keys manifest
example/1-network/genesis/*	Updates example configuration with XMSS keys and manifest

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-11-26T09:42:16Z

src/types/state.hpp

    ValidatorIndex validatorCount() const {
-      return config.num_validators;
+      return validators.size();


The validatorCount() method now returns validators.size() instead of config.num_validators. However, this could be problematic if the validators vector is not properly initialized or if it's empty. In scenarios where the state is constructed but validators haven't been populated yet, this would return 0, potentially causing issues in code that depends on validator counts. Consider adding validation to ensure validators is populated, or documenting the initialization requirements clearly.

Copilot · 2025-11-26T09:42:17Z

src/types/signed_attestation.hpp

+    auto payload = sszHash(attestation);
    return SignedAttestation{.message = attestation, .signature = {}};


The sszHash(attestation) is computed but assigned to payload which is then not used in the stub implementation. This unused variable suggests incomplete implementation. Either the payload should be used for actual signing, or it should be removed if this is intentionally a stub.

Copilot · 2025-11-26T09:42:17Z

src/blockchain/fork_choice.cpp

                     "Failed to produce block for slot {}: {}",
                     current_slot,
                     res.error());
+            time_ += 1;


Inconsistent time increment logic. When produceBlockWithSignatures fails (line 625), the code increments time and continues (line 630). However, the normal flow already increments time at line 705. This means that when block production fails, time is incremented twice in one iteration - once at line 630 and once at line 705. This will cause the node to skip an interval, which could lead to timing issues and missed attestation opportunities.

Suggested change

time_ += 1;

Copilot · 2025-11-26T09:42:17Z

src/blockchain/state_transition_function.cpp

+    for (size_t i = 0; i < registry->allValidatorsIndices().size(); ++i) {
+      auto opt_pubkey = validator_keys_manifest->getXmssPubkeyByIndex(i);
+      if (not opt_pubkey) {
+        continue;
+      }
+      result.validators.push_back(Validator{.pubkey = *opt_pubkey});
+    }


The genesis state generation iterates through validator indices using registry->allValidatorsIndices().size() (line 74) but then looks up the public key by that loop index i (line 75). This assumes that validator indices are contiguous starting from 0, which may not be the case. If allValidatorsIndices() returns a set like {5, 10, 15}, the code will look up indices 0, 1, 2 instead of 5, 10, 15, resulting in missing or incorrect validators. The loop should iterate over the actual indices returned by allValidatorsIndices(), not just its size.

Copilot · 2025-11-26T09:42:18Z

tests/unit/crypto/xmss_provider_test.cpp

+
+  // Check reasonable sizes
+  EXPECT_GT(keypair.public_key.size(), 0);
+  EXPECT_GT(keypair.private_key.size(), 0);


Similar to line 30, this test checks keypair.private_key.size() on line 34, but XmssPrivateKey is a std::shared_ptr which doesn't have a size() method. This test will not compile.

Copilot · 2025-11-26T09:42:20Z

src/blockchain/fork_choice.cpp

    return result;
  }

+


[nitpick] Empty line added at line 710. While not harmful, this extra blank line is inconsistent with the surrounding code formatting and should be removed.

Suggested change

Copilot · 2025-11-26T09:42:20Z

src/blockchain/state_transition_function.cpp

+    for (auto i = num_empty_slots; i > 0; --i) {
      state.historical_block_hashes.push_back(kZeroHash);
+    }
+
+    // Build new justified slots list
+    //
+    // Genesis block is always justified
+    state.justified_slots.push_back(is_genesis_parent);
+
+    // Mark empty slots as not justified
+    for (auto i = num_empty_slots; i > 0; --i) {


The loop variable i is declared as auto which will deduce to uint64_t (type of num_empty_slots). However, the loop counts down with --i and checks i > 0. If num_empty_slots is 0, the loop won't execute, which is correct. However, using an unsigned type in a countdown loop is a code smell - if the logic ever changes and i reaches 0 and then decrements, it will wrap around to the maximum value. Consider using a signed type or restructuring the loop to count up instead.

Copilot · 2025-11-26T09:42:20Z

src/app/configuration.cpp

+    return xmss_keypair_.value();
+  }
+
+


[nitpick] Extra blank line at line 68. This creates inconsistent spacing in the method implementation.

Suggested change

Copilot · 2025-11-26T09:42:20Z

src/types/config.hpp

-    SSZ_CONT(num_validators, genesis_time);
-    bool operator==(const Config &) const = default;
+    SSZ_CONT(genesis_time);
+


Trailing whitespace on line 18. This should be removed for code cleanliness.

Copilot · 2025-11-26T09:42:21Z

src/blockchain/impl/validator_registry_impl.cpp

+  ValidatorRegistry::ValidatorIndices
+  ValidatorRegistryImpl::allValidatorsIndices() const {
+    ValidatorIndices all_indices;
+    all_indices.reserve(index_to_node_.size());


The reserve call on line 92 is inefficient for a set container. ValidatorIndices appears to be a set (based on the insert call on line 94), and sets don't support reserve() since they're typically implemented as red-black trees. This will either fail to compile or the reserve call has no effect. If ValidatorIndices is actually an unordered_set, then reserve would work but the size hint would be wrong since we're reserving based on index_to_node_.size() but then inserting into the set. Sets automatically handle capacity, so this line should be removed.

Suggested change

all_indices.reserve(index_to_node_.size());

// reserve() is not needed for set containers

turuslan · 2025-11-26T10:00:37Z

src/types/signed_attestation.hpp

  /**
   * Stub method to sign vote.
   */
  inline SignedAttestation signAttestation(Attestation attestation) {


Not used, can be removed

Signed-off-by: turuslan <[email protected]>

kamilsa added 25 commits November 20, 2025 17:45

Refactor config and validator handling

0a48ce1

remove num_validators, update genesis_time, and enhance genesis state generation

Replace vote jargon with attestation leanSpec#91

736eeb7

Enhance attestation processing and comments leanSpec#108

3ed1ef5

Rename advanceTime to onTick leanSpec#134

af8d532

Introduce JUSTIFICATION_LOOKBACK_SLOTS leanSpec#131

76ad04b

Add early return for empty justifications in getJustifications function

c769b19

Rename latest_new_votes to latest_new_attestations in fork_choice.cpp

e3ee432

Add qdrvm-crates dependency and update CMake configuration

6e916b2

Implement XMSS provider with key generation, signing, and verification

f7d15e7

Update to poseidon2

218a8a5

Add KeyStore implementation for XMSS key management

a680716

Implement XMSS keypair loading and configuration management

60bbaf3

Update max secret key size buffer to 200 MB in XMSS provider implemen…

ece13c5

…tation

Add ValidatorKeysManifest implementation for managing validator keys

b19c3dc

52 bytes XMSS public key

5b9f0bf

Add mock implementations for ValidatorKeysManifest and XmssProvider

d354350

Rename vote-related methods to attestation for clarity and update moc…

1dc7aff

…k methods in ValidatorKeysManifestMock

Refactor XMSS signature: remove epoch, 3116 bytes

fd96f9e

Integrate hash-sig into fork-choice

9f9acca

Cleanup and more doc leanSpec#146

dce2b29

Refactor processBlockBodyAttestations leanSpec#153

999d8cc

Refactor block header processing leanSpec#154

35d2987

Remove processProposerAttestation leanSpec#155

0776305

remove processSlot leanSpec#163

1290df7

Remove getLatestJustified leanSpec#162

0230596

kamilsa force-pushed the feature/c-hash-sig-rebased branch from 2a1f827 to 0230596 Compare November 25, 2025 09:59

kamilsa added 4 commits November 25, 2025 15:03

More docs and small refactoring leanSpec#177

8d8796e

Align process attestations with intended specs leanSpec#160

e3881d2

Refactor leanSpec#178

8cb10a9

rm useless getJustifications leanSpec#179

7b53484

kamilsa and others added 4 commits November 25, 2025 15:53

Enhance documentation and comments leanSpec#183

c1f519b

Fix tests

164c095

refactor xmss

8b1f5d0

Signed-off-by: turuslan <[email protected]>

Make jsons with keys single line

005ec27

kamilsa marked this pull request as ready for review November 26, 2025 09:35

kamilsa requested review from Copilot, turuslan and xDimon November 26, 2025 09:35

Copilot started reviewing on behalf of kamilsa November 26, 2025 09:36 View session

Copilot finished reviewing on behalf of kamilsa November 26, 2025 09:39

Copilot AI reviewed Nov 26, 2025

View reviewed changes

turuslan approved these changes Nov 26, 2025

View reviewed changes

turuslan and others added 13 commits November 26, 2025 16:03

fix test

e8d2d16

Signed-off-by: turuslan <[email protected]>

Add helper function to create state with a single validator

a3c292b

Add produceAttestationData helper leanSpec#188

867dcee

Add break leanSpec#188

4199689

Implement LMD GHOST algorithm leanSpec#190

e70ecda

validator index

df2b524

Signed-off-by: turuslan <[email protected]>

remove unused vote

dc7cc1b

Signed-off-by: turuslan <[email protected]>

lmd ghost documentation and simplify leanSpec#192

ae7ef62

Refactor updateHead leanSpec#194

f5da763

Return error for invalid block signatures in fork choice

ec9211e

update and refactor xmss

e710d52

Signed-off-by: turuslan <[email protected]>

Remove stub method for signing votes in SignedAttestation

764abf9

Update qdrvm-crates to v1.0.6

1cc4b06

kamilsa merged commit df19a98 into master Dec 2, 2025
2 checks passed

kamilsa deleted the feature/c-hash-sig-rebased branch December 2, 2025 10:58

		auto payload = sszHash(attestation);
		return SignedAttestation{.message = attestation, .signature = {}};

	all_indices.reserve(index_to_node_.size());
	// reserve() is not needed for set containers

Hash sig #34

Hash sig #34

Uh oh!

Conversation

kamilsa commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

turuslan Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kamilsa commented Nov 21, 2025 •

edited

Loading