47-49: **Validate consistent use of the truncated BLS key as a 32-byte symmetric key **

Here, the code converts the public key into a byte vector, then truncates it to 32 bytes. Confirm this key derivation follows a secure practice (e.g., applying a KDF). Also ensure that future code reading expects only the first 32 bytes.

83-85: **Same key truncation logic as above **

100-102: **Same key truncation logic as above **

11-15: **Documentation clarity **

The added comment clearly states that the basic scheme is assumed for all masternodes and that serialization uses the Basic Scheme. This is a good step toward preventing misuse or confusion about which scheme is in effect.

Line range hint 209-215: **Atomic legacy scheme toggle **

Switching bls_legacy_scheme to false here relies on std::atomic for thread safety. Verify that no race conditions arise if multiple threads process blocks concurrently. Also confirm that flipping the global scheme immediately is safe for all ongoing operations.

Line range hint 230-236: **Reverting to legacy scheme on block undo **

Reverting bls_legacy_scheme when undoing the activation block is logical, but confirm that reorgs or partial replays won't lead to inconsistent scheme states. Consider how concurrency or partial failures might affect this toggle.

11-11: **Header inclusion check **

Including <bls/bls_ies.h> is appropriate for the newly introduced encryption methods. Confirm no redundant includes if performance or compile-times become concerns.

214-214: Conditionally switch registrar update method based on softfork state
Using 'v19' activation status to decide between update_registrar and update_registrar_legacy is a neat solution. Please ensure you test both branches in your functional tests, so that the older path is not overlooked.

87-88: Use of DeploymentActiveAfter to determine legacy BLS usage
Initializing m_use_legacy_bls with !DeploymentActiveAfter(...) is clear for deciding which scheme to use. However, ensure that reorg or varying block heights do not require a dynamic check or re-evaluation of this field. Consider verifying or documenting that the chosen block index won’t shift during runtime if a reorg happens.

219-219: Consistent signature generation with legacy parameter
Replacing direct calls to Sign(qc.GetSignHash()) with Sign(..., m_use_legacy_bls) aligns well with the global toggle. This looks correct.

531-531: Usage of legacy BLS toggle
Continuing the same pattern of calling Sign(...) with m_use_legacy_bls is consistent and straightforward.

725-725: Signature assignment with the correct scheme
Utilizing m_use_legacy_bls for qj.sig helps maintain uniform handling of the BLS scheme.

1015-1017: Potential scheme mismatch
Here, qc.sig is signed with use_legacy_bls, while qc.quorumSig is signed via bls::bls_legacy_scheme.load(). Verify that these two scheme indicators cannot drift out of sync in certain edge cases. A mismatch could produce invalid signatures.

1020-1022: Index bound check when mutating signature buffer
Modifying buf[5]++ helps simulate a corrupted signature. Ensure this index is always safe (e.g., signature length ≥ 6 bytes).

1024-1026: Quorum signature corruption simulation
Similar concern with buf[5]++. Confirm that the signature is always large enough so this index never goes out of range.

200-205: Hardcoding modOrder flag to false
Previously, there was a parameter for the secret key scheme. Now we default to false within ParseBLSSecretKey. Although simpler, confirm that ignoring the modOrder parameter does not lead to incorrect behavior when dealing with certain BLS keys.

310-317: Introducing the use_legacy option for special TX signing
Adding the bool use_legacy parameter clarifies which scheme is used. Ensure code that calls SignSpecialTxPayloadByHash thoroughly sets this flag and that tests cover both new and legacy paths.

657-657: Extracting use_legacy from incoming parameter
Defining use_legacy by specific_legacy_bls_scheme is straightforward. Double-check if the rest of the code references only use_legacy so no mismatch arises.

Line range hint 1306-1307: Dynamic selection of BLS generation based on v19
Conditionally calling bls('generate') or bls('generate', True) looks correct. Ensure you have coverage for both the pre- and post-v19 scenarios in your tests.

Line range hint 1340-1340: Register vs. register_legacy usage
Using 'register' if v19_active else 'register_legacy' is consistent with the approach throughout the codebase. Please confirm that any edge case around the activation transition is tested.

54-55: Confirm that forcing basic BLS does not impact backward compatibility
By invoking mn_activeman.Sign(signHash, false), you're mandating basic BLS for all nodes. Ensure that no remaining code paths require a legacy signature or a transition phase for older nodes.

90-91: Usage of mnauth.sig.ToString(false)
The signature stringification now explicitly defaults to "basic BLS" output, which is consistent with the new scheme. No issues found, but ensure logs remain human-readable and clearly indicate the scheme used.

117-117: Verify that “false” is correct for legacy parameter
You’ve hard-coded false for this VerifyInsecure call, which aligns with a basic BLS scheme. Confirm that this is correct for all p2p interactions you expect during MNAUTH.

32-32: Signing with the second parameter set to false
You are now specifying the signing scheme (basic vs. legacy). Looks consistent with the updated method signature. No correctness issue found.

37-37: Generating invalid signatures
Changing to s.Sign(msgHashes[i], false) clarifies you’re using the basic BLS scheme even for invalid signatures. No problem found here.

74-75: Consistent usage of false in signature creation
Both lines use the updated parameter to enforce basic BLS. This is coherent with the rest of the code changes.

92-92: Signing in benchmark test
Switching to secKey.Sign(hash, false) allows verifying performance under the new scheme. Behavior appears correct.

291-291: Ensure proper initialization of the new flag.
The new bool m_use_legacy_bls field should be explicitly assigned in the constructor or via initialization to avoid any uninitialized state issues.

47-47: No issue found.
The signature logic appears straightforward, and test coverage is sufficient for this line.

66-66: Comment clarity.
The added comment is accurate regarding the second parameter usage in SetHexStr. No further changes needed.

70-71: Validating hex strings.
These checks properly ensure the string is valid hexadecimal. The error handling is correct.

75-76: Coverage of short hex string edge case.
The test scenario for short hex strings is appropriate, and the error condition is verified.

78-80: Coverage of long hex string edge case.
Ensures that overly long strings are handled correctly. No changes needed.

105-105: Signature creation.
The invocation of Sign with the legacy_scheme flag is consistent with the broader changes to handle legacy vs. basic BLS scheme.

147-147: Signature verification coverage.
Signing and verifying with aggregated keys is properly tested.

173-173: Aggregation logic verification.
This line is part of the process to aggregate signatures. Looks correct.

224-224: Secure aggregation approach.
Use of Sign and AggregateSecure is logically consistent.

267-267: Capturing global scheme state.
Capturing the current BLS scheme in a local variable is valid, ensuring consistency for subsequent calls.

274-274: Consistency in creating signatures.
This call uses the newly introduced legacy_scheme parameter. No issues noted.

280-280: Alternate signature creation.
Switching to a different key ensures negative test coverage. Looks correct.

385-385: Threshold signing.
The introduced parameter is consistent with the rest of the code.

402-402: Recovering threshold shares.
Checks each share’s validity, with consistent usage of the legacy scheme flag.

65-65: Confirm consistent signing approach.
This verifies signatures with false for the second parameter, matching the approach in Sign(...). Ensure no mismatch arises elsewhere.

104-104: Consistent verification parameter.
Again, passing false to match non-legacy usage is correct for these checks.

546-546: Updated expectation for default scheme.
The test verifying the scheme as "basic" aligns with the new BLS default.

558-560: Testing bls fromsecret with basic scheme.
Ensures the returned public key matches the basic scheme's expectation, confirming functional correctness.

566-566: Zero argument implies basic scheme.
Consistent with the changes that default to is_legacy = false.

579-579: Checking legacy scheme.
This call confirms that passing 1 triggers a legacy BLS signature.

582-582: Verifying no second parameter implies basic.
Aligned with default scheme expectations.

765-765: Explicitly specifying the scheme looks correct.
The call to secKey.Sign(msgHash, bls::bls_legacy_scheme.load()) clarifies the use of the legacy scheme for BLS signatures and aligns with the broader refactoring.

135-135: Validate consistent usage of the legacy scheme.
Including the legacy scheme parameter is correct. Verify that all dependent test cases account for the newly explicit bls::bls_legacy_scheme usage to prevent any signature mismatches.

1544-1544: Ensure legacy scheme parameter is consistently applied.
The updated signature creation with bls::bls_legacy_scheme.load() is clear. Confirm calls throughout the codebase also specify this scheme if intended, preventing mismatched configurations.

1340-1340: Validate usage of 'register' vs 'register_legacy' for future-proofing.
The inline ternary is appropriate here, but double-check that any interaction with other code (like future forks) won’t break if the legacy or non-legacy approach changes again. This is especially relevant given the reliance on softfork activation.

1422-1422: Ensure test coverage for the 'register_fund' vs 'register_fund_legacy' call.
It is crucial to confirm that the new logic for choosing between the two registration methods addresses corner cases, including incomplete migrations and wallet states.

1425-1425: Check consistency of the 'register' vs 'register_legacy' calls.
Using the inline conditional for different protx commands looks correct, but validating in integration tests that each path behaves as expected post-v19 is recommended.

11-11: LGTM: BLS IES header inclusion

The addition of bls/bls_ies.h aligns with the PR's objective to refactor the BLS legacy scheme and supports the new encrypted contributions functionality.

Line range hint 250-253: LGTM: WriteEncryptedContributions implementation

The method follows the established pattern of other Write* methods in the class and maintains proper const correctness.

Line range hint 28-30: LGTM: Clean integration of encrypted contributions

The new encrypted contributions are properly integrated into the existing cleanup mechanism, maintaining backward compatibility and following the established patterns for database operations.

Also applies to: 400-450

87-88: LGTM: Proper initialization of BLS scheme based on V19 deployment status.

The initialization of m_use_legacy_bls correctly determines the BLS scheme based on V19 deployment status, which aligns with the PR objectives.

219-219: LGTM: Consistent BLS scheme usage in contribution signatures.

The signature generation now correctly uses the instance-specific BLS scheme flag, ensuring consistency with V19 deployment status.

531-531: LGTM: Consistent BLS scheme usage in complaint signatures.

The signature generation now correctly uses the instance-specific BLS scheme flag, ensuring consistency with V19 deployment status.

725-725: LGTM: Consistent BLS scheme usage in justification signatures.

The signature generation now correctly uses the instance-specific BLS scheme flag, ensuring consistency with V19 deployment status.

12-12: LGTM: Improved type safety in EncryptBlob signature

The change from template parameter to specific vector type improves type safety and makes the interface more explicit.

83-83: LGTM: Consistent Basic scheme usage

The change maintains consistency with the Basic BLS scheme usage for key derivation across the codebase.

100-100: LGTM: Symmetric Basic scheme usage in encryption/decryption

The change ensures symmetric behavior between encryption and decryption operations by consistently using the Basic scheme.

47-47: ⚠️ Potential issue

Verify symmetric key derivation changes

The change to pk.ToByteVector(false) enforces the Basic BLS scheme for key derivation. This is a breaking change that affects symmetric key generation.

Run this script to verify consistent usage across the codebase:

#!/bin/bash
# Description: Check for any remaining unspecified ToByteVector calls in BLS-related code
# that might need the explicit legacy parameter

# Search for ToByteVector calls without explicit legacy parameter
rg "ToByteVector\(\s*\)" --type cpp

# Search for all ToByteVector calls to compare
rg "ToByteVector\(" --type cpp

#!/bin/bash
# Search for BLS-related ToByteVector calls to verify consistency
rg "ToByteVector\(.*\)" --type cpp src/bls/

# Search for BLS key derivation related code
ast-grep --pattern 'CBLSPublicKey::ToByteVector($_)'

#!/bin/bash
# Search for BLS IES encryption/decryption implementation
rg -A 10 "CBLSIESEncryptedBlob::(Encrypt|Decrypt)" --type cpp

# Search for BLS IES tests
rg "TEST.*bls.*ies" -i --type cpp

#!/bin/bash
# Search for all DHKeyExchange calls to verify BLS scheme consistency
rg "DHKeyExchange" --type cpp -B 2 -A 2

# Search for other BLS IES encryption/decryption methods
rg "CBLSIESEncryptedBlob::" --type cpp -B 2 -A 2

15-15: LGTM: Clean import addition

The addition of softfork_active import is appropriate for the new conditional logic.

229-229: LGTM: Clean conditional BLS key generation

The conditional logic for BLS key generation appropriately handles both legacy and new schemes based on v19 activation status.

257-257: LGTM: Consistent ProTx registration handling

The conditional logic for ProTx registration commands maintains consistency with the BLS scheme changes and properly handles both legacy and new cases.

Also applies to: 273-273

214-214: LGTM: Consistent registrar update handling

The conditional logic for registrar updates follows the same clean pattern used throughout the file and properly handles both legacy and new cases.

Also applies to: 291-291

310-317: LGTM: Clean implementation of BLS scheme selection

The template-based implementation with explicit legacy scheme control is well-designed and maintainable.

1704-1729: LGTM: Well-documented BLS generate command

The changes to the bls generate command are well-implemented with clear documentation and proper handling of both legacy and new schemes.

1736-1764: LGTM: Consistent BLS fromsecret implementation

The changes to the bls fromsecret command maintain consistency with the bls generate command and provide clear documentation.

11-11: LGTM!

The BLS IES header inclusion is properly placed and necessary for the encrypted contributions functionality.

87-88: LGTM!

The initialization of m_use_legacy_bls correctly determines the BLS scheme based on V19 deployment status.

219-219: LGTM!

The signature generation correctly uses the instance-specific m_use_legacy_bls flag, aligning with the deglobalization effort.

531-531: LGTM!

The complaint signature generation consistently uses the instance-specific BLS scheme flag.

725-725: LGTM!

The justification signature generation maintains consistency by using the instance-specific BLS scheme flag.

Line range hint 87-1025: Excellent consistency in BLS scheme deglobalization

The changes systematically replace the global BLS scheme flag with the instance-specific m_use_legacy_bls across all signature operations in the DKG session. This consistent implementation aligns perfectly with the PR's deglobalization objectives.

1015-1025: LGTM with a note about error simulation

The code consistently uses the instance-specific BLS scheme flag for both signature generation and byte vector operations. The signature manipulation is intentional for testing error scenarios.

#!/bin/bash
# Verify that error simulation is only used in test code
rg "ShouldSimulateError.*COMMIT_LIE" --type cpp

#!/bin/bash
# Check if this is in test code by looking for test-related indicators
rg "ShouldSimulateError.*COMMIT_LIE" -B 10 -A 10

# Also check for test-related includes or definitions
rg "ShouldSimulateError" --type cpp -B 5

Line range hint 1-120: Verify encryption path coverage after method removals.

The removal of multiple Encrypt methods simplifies the API, but we should verify that all encryption use cases are still properly covered by the remaining implementation.

Let's verify the usage patterns:

#!/bin/bash
# Search for all encryption method calls in the codebase
echo "Searching for encryption method calls..."
rg "Encrypt\(" --type cpp

# Search for potential encryption-related tests
echo "Searching for related test files..."
fd -e cpp -e h "test.*bls.*ies"

#!/bin/bash
# Search for BLS IES test files and their content
echo "Searching for BLS IES tests..."
rg -l "CBLSIESMultiRecipientBlobs|CBLSIESEncryptedObject" --type cpp

echo -e "\nChecking test content..."
rg "TEST.*bls.*ies" --type cpp -B 2 -A 2

echo -e "\nChecking for removed Encrypt methods in git history..."
git log -p --no-merges -- src/bls/bls_ies.h src/bls/bls_ies.cpp | rg "^[-+].*Encrypt.*\("

#!/bin/bash
# Check how BLS IES encryption is used in the codebase
echo "Checking BLS IES usage in masternode..."
rg -A 5 -B 5 "CBLSIESMultiRecipientBlobs|CBLSIESEncryptedObject" src/masternode/node.cpp

echo -e "\nChecking BLS IES usage in LLMQ..."
rg -A 5 -B 5 "CBLSIESMultiRecipientBlobs|CBLSIESEncryptedObject" "src/llmq/*.cpp"

echo -e "\nChecking current implementation of encryption methods..."
rg "bool.*Encrypt" src/bls/bls_ies.cpp

#!/bin/bash
# Check LLMQ usage with correct path
echo "Checking BLS IES usage in LLMQ..."
rg -A 5 -B 5 "CBLSIESMultiRecipientBlobs|CBLSIESEncryptedObject" src/llmq/

echo -e "\nChecking DKG session implementation..."
rg -A 10 "contributions->InitEncrypt|contributions->Encrypt" src/llmq/dkgsession.cpp

echo -e "\nChecking for any encryption-related tests..."
fd -e cpp "test" | xargs rg "CBLSIESMultiRecipientBlobs|CBLSIESEncryptedObject"

Line range hint 12-19: Good simplification of the EncryptBlob interface.

Removing the template parameter in favor of a concrete type improves type safety and makes the interface more explicit.

47-47: Verify implications of explicit basic scheme usage in key derivation.

The change to use ToByteVector(false) explicitly specifies the basic scheme for key derivation. This is a security-critical change that affects the symmetric key generation process.

Let's verify the consistency of this change:

Also applies to: 83-83, 100-100

#!/bin/bash
# Search for all ToByteVector calls to ensure consistent usage
echo "Checking ToByteVector usage patterns..."
rg "ToByteVector\(" --type cpp -A 1 -B 1

# Search for any tests verifying key derivation
echo "Searching for key derivation tests..."
rg "DHKeyExchange.*ToByteVector" --type cpp

#!/bin/bash
# Search for key derivation related code in bls_ies.cpp
echo "Checking key derivation in bls_ies.cpp..."
rg "symKey|ToByteVector" src/bls/bls_ies.cpp -B 2 -A 2

# Search for tests related to BLS IES encryption
echo "Checking BLS IES tests..."
rg "bls_ies.*test" -l

#!/bin/bash
# Search for BLS scheme related code and tests
echo "Checking BLS scheme usage..."
rg "legacy.*scheme|basic.*scheme" src/bls/ -B 2 -A 2

# Search for BLS ToByteVector implementations
echo "Checking BLS ToByteVector implementations..."
ast-grep --pattern 'ToByteVector($_, $_) { $$$ }'