Unified Crypto refactor #281
Conversation
+ refactor tx validation signature validation + refactor validation signatures + delete old secretary manager class + refactor
- Introduced methods for adding and removing PQC identities in GCRIdentityRoutines. - Implemented PQC payload verification in IdentityManager. - Updated handleIdentityRequest to support PQC identity assignments and removals.
- Added validation for adding and removing PQC identities, ensuring proper data structure and existence checks. - Updated StoredIdentities type to include PQC identities structure. - Improved error handling for missing or duplicate identities during operations.
- Refactored the `verifyWeb2Proof` function to include error handling and improved success messaging. - Updated `GCRIdentityRoutines` to handle identities as objects, including address, signature, public key, and timestamp. - Modified `IdentityManager` to accept the sender's ed25519 address for payload verification. - Adjusted `handleIdentityRequest` to correctly process transaction data and sender information. - Improved type definitions for stored identities to accommodate new structure.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces a comprehensive refactor of cryptographic operations throughout the codebase, transitioning from direct Ed25519 key usage and synchronous cryptography to a unified, asynchronous API ( Changes
Sequence Diagram(s)sequenceDiagram
participant NodeA as Node A
participant NodeB as Node B
participant ucrypto as Unified Crypto
NodeA->>ucrypto: sign(message, signingAlgorithm)
ucrypto-->>NodeA: signature { type, data }
NodeA->>NodeB: send { message, signature: { type, data }, publicKey }
NodeB->>ucrypto: verify({ algorithm, message, publicKey, signature })
ucrypto-->>NodeB: verification result (true/false)
sequenceDiagram
participant User
participant Node
participant GCR as GCRIdentityRoutines
participant Repo as GCRMainRepository
User->>Node: Request PQC identity add (batch)
Node->>GCR: applyPqcIdentityAdd(editOperation, Repo, simulate)
GCR->>Repo: Validate and store PQC identities
Repo-->>GCR: Save result
GCR-->>Node: Success/failure response
Possibly related issues
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 31
🔭 Outside diff range comments (2)
src/features/web2/Web2RequestManager.ts (1)
127-143:⚠️ Potential issueHashing mismatch makes every attestation invalid
validateWeb2RequestAndResulthashes{ request.raw, result }, but
verifyWeb2RequestAndResultrecomputes the hash only onrequest.raw.That guarantees
hashIsValid === falsewheneverresultis non-empty.- const stringifiedContent = JSON.stringify(this.dahr.web2Request.raw) + const stringifiedContent = JSON.stringify({ + request: this.dahr.web2Request.raw, + result: this.dahr.web2Request.result, + })src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
512-538:⚠️ Potential issueQuery parameters interpolated in JSON path – will not bind
Postgres placeholders (
:chain,:subchain) cannot be used inside JSON
operators. The query is executed verbatim, so the planner sees the string
:chain, not its bound value, and the query always returns no rows.You need string interpolation after sanitising the identifiers, or a
different approach (e.g.jsonb_extract_path_text) with constant column paths.const query = ` EXISTS ( SELECT 1 FROM jsonb_array_elements(gcr.identities->'xm'->'${chain}'->'${subchain}') AS xm_id WHERE xm_id->>'address' = :address ) ` qb.where(query, { address: addressToCheck })Be sure to validate
chain/subchainagainst an allow-list first.
[security]
♻️ Duplicate comments (1)
src/libs/peer/PeerManager.ts (1)
296-305: Same API-shape concern forucrypto.signas increateBlock.Double-check that the positional-parameter overload is valid; otherwise
signaturemay be undefined.
🧹 Nitpick comments (29)
.env.example (1)
1-5: Consider security best practices for credential management.While this example file appropriately provides empty placeholders, consider documenting secure credential management alternatives:
- Use credential management tools (HashiCorp Vault, AWS Secrets Manager)
- Implement OAuth flows instead of storing passwords
- Use GitHub tokens with minimal required scopes
Consider adding security comments to the example:
+# Security Note: Avoid storing passwords in environment variables in production +# Consider using OAuth, credential managers, or secure secret storage TWITTER_USERNAME= TWITTER_PASSWORD= TWITTER_EMAIL= +# Generate tokens with minimal required scopes GITHUB_TOKEN=src/libs/network/manageNodeCall.ts (1)
23-23: Side-effect-only import – consider tree-shaking friendly form
uint8ArrayToHexis imported solely for the"getPeerIdentity"branch. Keeping it here is fine, but if bundle size matters, consider dynamic import or exposingpublicKeyHexdirectly from shared state.src/libs/abstraction/web2/parsers.ts (1)
43-44: Add validation for SigningAlgorithm casting.The type casting
splits[2] as SigningAlgorithmassumes the input is always a valid algorithm. Consider adding validation to ensure robustness.return { message: splits[1], - type: splits[2] as SigningAlgorithm, + type: splits[2] as SigningAlgorithm, // TODO: Validate algorithm signature: splits[3], }Alternatively, add runtime validation:
const algorithm = splits[2] as SigningAlgorithm; if (!Object.values(SigningAlgorithm).includes(algorithm)) { throw new Error(`Invalid signing algorithm: ${algorithm}`); }src/libs/abstraction/web2/github.ts (1)
91-94: Use class name instead ofthisin static context.The static analysis correctly identifies that using
thisin a static context can be confusing. For better clarity, use the class name directly.Apply this diff to improve code clarity:
static async getInstance() { if (!this.instance) { this.instance = new this() - await this.instance.login() + await GithubProofParser.instance.login() }Additionally, for consistency with the instantiation pattern:
static async getInstance() { - if (!this.instance) { - this.instance = new this() - await this.instance.login() + if (!GithubProofParser.instance) { + GithubProofParser.instance = new GithubProofParser() + await GithubProofParser.instance.login() } - return this.instance + return GithubProofParser.instance }🧰 Tools
🪛 Biome (1.9.4)
[error] 91-91: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 92-92: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 92-92: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 93-93: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
src/features/web2/handleWeb2.ts (1)
101-101: Address the TODO comment regarding attestation validation.The comment indicates that
web2ResultIsValidnow returns a Promise, but the attestation validation logic appears to be incomplete with the TODO comment below.Would you like me to help implement the attestation validation logic for non-original RPC requests? I can generate the missing implementation that properly validates attestations using the async
web2ResultIsValidmethod.src/libs/abstraction/index.ts (1)
1-1: Remove unused import.The
Cryptographyimport is no longer used after the migration toucrypto.-import Cryptography from "../crypto/cryptography" import { GithubProofParser } from "./web2/github"src/libs/network/manageHelloPeer.ts (1)
8-8: Remove unused import.The
Cryptographyimport is no longer used after migrating toucrypto.import * as forge from "node-forge" -import Cryptography from "../crypto/cryptography" import { SyncData } from "../peer/Peer"src/libs/consensus/v2/routines/manageProposeBlockHash.ts (1)
28-28: Avoid repeatinggetSharedState.publicKeyHex.The same literal is assigned to
response.responsefour times.
Stash it in aconst ourKey = getSharedState.publicKeyHex;to reduce
property-lookups and keep the code DRY.- response.response = getSharedState.publicKeyHex + const ourKey = getSharedState.publicKeyHex + response.response = ourKeyAlso applies to: 50-50, 60-60, 101-101
src/libs/consensus/v2/routines/createBlock.ts (1)
51-52: Optional: clarifyvalidation_datatyping.The “! Define a decent type for validation_data” comment hints at tech
debt. Consider introducing an explicit interface instead of the loose
object literal.src/libs/peer/PeerManager.ts (1)
100-101: 👍 Extra logging is harmless but remember to drop verboseconsole.log.In production these console calls add noise;
log.debugalready exists.src/libs/network/routines/transactions/handleIdentityRequest.ts (1)
31-36: Clarify the sender parameter usage for xm_identity_assign.The comment mentions that sender can be either ed25519 or PQC address, but the code doesn't seem to use the
senderparameter forxm_identity_assign. TheverifyPayloadmethod only takes the payload parameter.Consider clarifying the comment or verify if the sender parameter should be passed:
case "xm_identity_assign": - // NOTE: Sender here is the ed25519 address coming from the transaction body - // because the xm identity tx can be signed with both ed25519 and pqc. - // The sender address here will be the message to verify using the signature in the payload. + // NOTE: XM identity verification uses the signed data in the payload for verification return await IdentityManager.verifyPayload( payload.payload as InferFromSignaturePayload, )src/libs/blockchain/gcr/gcr_routines/identityManager.ts (2)
190-192: Fix static analysis warning: Replace 'this' with class name.The static analysis tool correctly identifies that using
thisin a static context is confusing and should be replaced with the class name.static async getPQCIdentity(address: string) { - return await this.getIdentities(address, "pqc") + return await IdentityManager.getIdentities(address, "pqc") }🧰 Tools
🪛 Biome (1.9.4)
[error] 191-191: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
128-156: Verify the PQC verification logic and error handling.The PQC verification method uses Ed25519 for signature verification, which seems correct for the current implementation. However, consider these improvements:
- The message encoding uses
payload.addressdirectly - ensure this matches the signing process- Error handling could be more descriptive about which specific payload failed
Consider enhancing error reporting:
if (!verified) { return { success: false, - message: `${payload.algorithm} payload could not be verified`, + message: `${payload.algorithm} payload for address ${payload.address} could not be verified`, } }src/model/entities/types/IdentityTypes.ts (1)
17-28: Consider consolidating PQC identity interfaces.The separation between
SavedPqcIdentityandPqcIdentityEditseems artificial sincePqcIdentityEditonly adds thealgorithmfield. Consider if this separation is necessary or if a single interface with optional fields would be simpler.Alternative approach:
-export interface SavedPqcIdentity { - address: string - signature: string - timestamp: number -} - -/** - * The PQC identity GCR edit operation data - */ -export interface PqcIdentityEdit extends SavedPqcIdentity { - algorithm: string -} +export interface PqcIdentity { + address: string + signature: string + timestamp: number + algorithm?: string // Optional for stored identities, required for operations +}src/libs/blockchain/routines/validateTransaction.ts (4)
18-26: Remove danglingCryptographyimport
Cryptography(line 18) is no longer referenced after the migration toucrypto; keeping it will trigger linter/TS errors (no-unused-vars).-import Cryptography from "src/libs/crypto/cryptography"
54-60: Avoid per-call identity fetch; cache the RPC public key
ucrypto.getIdentity()performs async I/O every timeconfirmTransactionruns.
If this routine is executed for every transaction, the extra await adds measurable latency.
Store the public key once at process start (or ingetSharedState) and reuse it.
90-95: Unusedconfirmationvariable
confirmationis destructured but never used – dead code and potential lint failure.
Simply omit it from the destructuring or use the value.-const { confirmation, message, success: verified } = - await Transaction.confirmTx(tx, sender) +const { message, success: verified } = + await Transaction.confirmTx(tx, sender)
133-136:privateKeyparameter is now obsolete
defineGasno longer touchesprivateKey; keeping it misleads callers.-async function defineGas( - tx: Transaction, - validityData: ValidityData, - privateKey: pki.PrivateKey, +async function defineGas( + tx: Transaction, + validityData: ValidityData,src/libs/peer/Peer.ts (2)
5-8: Prune unusedCryptographyimportAfter the
ucryptomigration,Cryptographyisn’t referenced.-import Cryptography from "../crypto/cryptography"
270-273: Skip empty auth headers when unauthenticatedWhen
isAuthenticated === false, the current implementation still transmitsidentity: ""andsignature: "". Many peers will treat this as malformed.
Set the headers conditionally instead.src/libs/network/manageConsensusRoutines.ts (2)
15-15: Remove unusedCryptographyimportNo reference remains after header-level verification was off-loaded.
-import Cryptography from "../crypto/cryptography"
219-225: Shard-membership rejection is correct – but consider 403 instead of 401
401implies “unauthenticated”; lack-of-membership is an authorization issue (403).
Minor but improves API semantics.src/features/web2/Web2RequestManager.ts (1)
180-193: Busy-wait loop can hog the event loop
quorumIsReacheduses a 100 ms polling loop withsetTimeout, which can block the event loop under high concurrency. Preferawait Waiter.wait(...)or an event-driven promise.src/libs/network/endpointHandlers.ts (1)
186-191: Avoid unnecessary async crypto call
ucrypto.getIdentity()hits WASM every time. You already expose the hex key viagetSharedState.publicKeyHex; converting once withhexToUint8Arrayis faster and avoids an await.src/libs/network/server_rpc.ts (1)
248-249: Stylistic rule violated (quotes)ESLint complains because the string literal uses single quotes whereas the
project rule enforces double quotes. Align with the rule to keep CI green:-server.get("/", () => new Response('{"message": "Hello, World!"}')) +server.get("/", () => new Response("{\"message\": \"Hello, World!\"}"))🧰 Tools
🪛 ESLint
[error] 248-248: Strings must use doublequote.
(quotes)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (2)
294-329: Earlyreturnaborts batch processing after first duplicate
applyPqcIdentityAddexits immediately when it finds an already–existing key.
This means that identities appearing after the duplicate are never checked
or added, surprising the caller.Collect duplicates and process the rest, then return an aggregated result:
- if (keyExists) { - return { - success: false, - message: `Identity already exists for algorithm ${algorithm}`, - } - } + if (keyExists) { + duplicates.push(`${algorithm}:${address}`) + continue + }…and finally:
return duplicates.length ? { success: false, message: `Already exists: ${duplicates.join(", ")}` } : { success: true, message: "PQC identities added" }
474-487: Use class name instead ofthisin static contextBiome flags
this.applyPqcIdentityAdd/this.applyPqcIdentityRemovein a
static method. Replace with the explicit class name for clarity:- case "pqcadd": - result = await this.applyPqcIdentityAdd( + case "pqcadd": + result = await GCRIdentityRoutines.applyPqcIdentityAdd((Repeat for the remove path.)
🧰 Tools
🪛 Biome (1.9.4)
[error] 474-474: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
[error] 481-481: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
src/libs/blockchain/transaction.ts (2)
465-479: Storingsignatureas stringified JSON – brittleSerialising nested structures into a single DB column makes querying and index
creation painful and can break on circular refs. Consider:signature_type: varchar, signature_data: textor a JSONB column if the database supports it.
At minimum, keep the structure binary-safe (Base64 / hex) and avoid
JSON.stringifyhere.
321-323: Verboseconsole.logstatements will flood production logs
confirmTx,validateSignature, etc. dump large objects on every call.
Replace with the existinglogutility at an appropriate level or
remove before shipping.🧰 Tools
🪛 Biome (1.9.4)
[error] 323-323: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.env.example(1 hunks).eslintrc.cjs(1 hunks)package.json(1 hunks)run(1 hunks)src/features/incentive/PointSystem.ts(2 hunks)src/features/web2/Web2RequestManager.ts(8 hunks)src/features/web2/dahr/DAHR.ts(1 hunks)src/features/web2/handleWeb2.ts(2 hunks)src/index.ts(2 hunks)src/libs/abstraction/index.ts(2 hunks)src/libs/abstraction/web2/github.ts(3 hunks)src/libs/abstraction/web2/parsers.ts(3 hunks)src/libs/abstraction/web2/twitter.ts(2 hunks)src/libs/blockchain/chain.ts(1 hunks)src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts(10 hunks)src/libs/blockchain/gcr/gcr_routines/identityManager.ts(4 hunks)src/libs/blockchain/gcr/gcr_routines/index.ts(1 hunks)src/libs/blockchain/mempool.ts(2 hunks)src/libs/blockchain/routines/validateTransaction.ts(6 hunks)src/libs/blockchain/transaction.ts(8 hunks)src/libs/blockchain/types/confirmation.ts(1 hunks)src/libs/consensus/v2/PoRBFT.ts(0 hunks)src/libs/consensus/v2/routines/broadcastBlockHash.ts(4 hunks)src/libs/consensus/v2/routines/createBlock.ts(2 hunks)src/libs/consensus/v2/routines/manageProposeBlockHash.ts(5 hunks)src/libs/consensus/v2/routines/secretary.ts(0 hunks)src/libs/consensus/v2/types/secretaryManager.ts(4 hunks)src/libs/identity/identity.ts(1 hunks)src/libs/network/endpointHandlers.ts(12 hunks)src/libs/network/manageConsensusRoutines.ts(6 hunks)src/libs/network/manageExecution.ts(3 hunks)src/libs/network/manageGCRRoutines.ts(1 hunks)src/libs/network/manageHelloPeer.ts(4 hunks)src/libs/network/manageNodeCall.ts(2 hunks)src/libs/network/routines/nodecalls/getPeerlist.ts(2 hunks)src/libs/network/routines/transactions/handleIdentityRequest.ts(2 hunks)src/libs/network/server_rpc.ts(12 hunks)src/libs/peer/Peer.ts(4 hunks)src/libs/peer/PeerManager.ts(6 hunks)src/libs/peer/routines/getPeerIdentity.ts(1 hunks)src/libs/utils/demostdlib/deriveMempoolOperation.ts(3 hunks)src/model/entities/Mempool.ts(2 hunks)src/model/entities/Transactions.ts(2 hunks)src/model/entities/types/IdentityTypes.ts(1 hunks)src/utilities/sharedState.ts(3 hunks)
💤 Files with no reviewable changes (2)
- src/libs/consensus/v2/PoRBFT.ts
- src/libs/consensus/v2/routines/secretary.ts
🧰 Additional context used
🧬 Code Graph Analysis (14)
src/libs/network/manageGCRRoutines.ts (1)
src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
IdentityManager(50-208)
src/libs/network/manageNodeCall.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/index.ts (1)
src/utilities/sharedState.ts (2)
getSharedState(222-224)publicKeyHex(95-101)
src/libs/blockchain/chain.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/utils/demostdlib/deriveMempoolOperation.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/network/routines/nodecalls/getPeerlist.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/consensus/v2/routines/manageProposeBlockHash.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/features/web2/handleWeb2.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/consensus/v2/routines/createBlock.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/peer/PeerManager.ts (2)
src/utilities/sharedState.ts (1)
getSharedState(222-224)src/libs/network/manageHelloPeer.ts (1)
HelloPeerRequest(12-20)
src/libs/consensus/v2/routines/broadcastBlockHash.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/consensus/v2/types/secretaryManager.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/peer/Peer.ts (1)
src/utilities/sharedState.ts (1)
getSharedState(222-224)
src/libs/blockchain/transaction.ts (4)
src/utilities/sharedState.ts (1)
getSharedState(222-224)src/libs/blockchain/types/confirmation.ts (1)
Confirmation(14-31)src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
IdentityManager(50-208)src/model/entities/types/IdentityTypes.ts (1)
SavedPqcIdentity(17-21)
🪛 Biome (1.9.4)
src/libs/abstraction/web2/github.ts
[error] 93-93: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
src/libs/blockchain/gcr/gcr_routines/identityManager.ts
[error] 191-191: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
src/libs/blockchain/transaction.ts
[error] 127-127: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 132-132: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 142-142: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts
[error] 474-474: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
[error] 481-481: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
🪛 ESLint
src/libs/network/server_rpc.ts
[error] 248-248: Strings must use doublequote.
(quotes)
🔇 Additional comments (54)
.eslintrc.cjs (1)
42-43: LGTM: Naming convention update supports crypto refactor patterns.The change to allow leading and trailing underscores for variable-like identifiers is appropriate for the crypto refactor that introduces new data structures. This maintains camelCase requirements while providing flexibility for cryptographic field naming.
src/utilities/sharedState.ts (3)
4-4: LGTM: Crypto imports support unified refactor.The new imports for node-forge, SigningAlgorithm, and uint8ArrayToHex properly support the transition to unified cryptographic operations.
Also applies to: 12-13
24-24: LGTM: Explicit signing algorithm specification.Setting the default signing algorithm to "ed25519" provides clear algorithm specification for the unified crypto system.
84-94: LGTM: Flexible keypair structure supports multiple crypto types.The keypair object appropriately accommodates different key formats (Uint8Array, RSA, Ed25519) needed for the crypto refactor while maintaining type safety.
package.json (1)
51-51:Details
❓ Verification inconclusive
Verify the twitter-scraper version for compatibility and security.
The version bump from
^0.16.6to^0.17.0should be validated for breaking changes and security issues.
🏁 Script executed:
#!/bin/bash # Description: Check twitter-scraper version and security advisories # Check if version exists curl -s https://registry.npmjs.org/@the-convocation/twitter-scraper | jq '.versions | has("0.17.0")' # Check for any known vulnerabilities npm audit --package-lock-only --json 2>/dev/null | jq '.vulnerabilities | to_entries[] | select(.key | contains("twitter-scraper"))'Length of output: 280
Verify
@the-convocation/twitter-scraperbump – no vulnerabilities found
- Version
0.17.0is published on npm.npm audit --package-lock-onlyreports no known security issues for this package.- Action: Review the dependency’s CHANGELOG or release notes for any breaking changes before merging.
Location to review:
package.json, line 51:
"@the-convocation/twitter-scraper": "^0.17.0"src/libs/peer/routines/getPeerIdentity.ts (1)
29-29: LGTM! Removal aligns with the unified crypto refactor.Removing the local identity retrieval and logging is consistent with the transition away from direct
getSharedState.identity.ed25519access to the new unified crypto module.src/libs/network/manageGCRRoutines.ts (1)
31-31: LGTM! Parameter change improves API consistency.Changing from
sendertoparams[0]is correct based on theIdentityManager.getIdentities(address: string, key?: string)method signature. This allows querying identities for any address, not just the sender.Also applies to: 36-36, 43-43
src/model/entities/Mempool.ts (2)
2-2: LGTM: Import statement simplified appropriately.The single-line import for
Transactionfrom the demosdk types package is clean and follows good import practices.
50-51: LGTM: Ed25519 signature column addition supports crypto refactor.The new nullable
ed25519_signaturevarchar column aligns well with the unified crypto refactor objectives. The nullable constraint ensures backward compatibility during the migration from the previous cryptographic implementation.src/features/web2/dahr/DAHR.ts (1)
89-89: LGTM: Proper async handling for cryptographic operations.The addition of
awaitcorrectly handles the asynchronous nature ofgetAttestedResultfollowing the migration to the unified async crypto API. This ensuresattestedResultis fully resolved before being spread into the return object.src/features/incentive/PointSystem.ts (1)
53-60: LGTM: Improved identity data structure handling.The renaming from
addressestoidentitiesand the corresponding property access change toidentity.addressbetter reflects the enhanced identity data structure. This aligns well with the broader refactor to support richer identity objects containing additional cryptographic metadata.src/libs/network/routines/nodecalls/getPeerlist.ts (2)
9-9: LGTM: Comment formatting improvement.Trimming trailing whitespace improves code cleanliness.
19-19: LGTM: Simplified access to canonical public key hex.The change from
getSharedState.identity.ed25519.publicKey.toString("hex")togetSharedState.publicKeyHexsimplifies the code by using the canonical hex-encoded public key property. This aligns well with the unified crypto refactor objectives and improves maintainability.src/libs/identity/identity.ts (1)
92-95: Good optimization: Removed unnecessaryawait.Since
this.publicIPis already a resolved string value, theawaitoperator was redundant. This change improves performance slightly while maintaining the same return behavior.src/libs/network/manageExecution.ts (3)
44-47: Consistent parameter forwarding.The sender parameter is properly forwarded to the validation handler.
🧰 Tools
🪛 Biome (1.9.4)
[error] 44-47: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
71-74: Consistent parameter forwarding.The sender parameter is properly forwarded to the execution handler.
13-16:Details
✅ Verification successful
LGTM: Added sender parameter for identity-aware processing.
The addition of the
senderparameter aligns with the broader cryptographic refactor to support identity-aware transaction processing.Verify that the called handler methods accept the new
senderparameter:
🏁 Script executed:
#!/bin/bash # Description: Verify that handleValidateTransaction and handleExecuteTransaction accept sender parameter # Search for the method signatures to confirm they accept the sender parameter ast-grep --pattern 'handleValidateTransaction($_, $sender) { $$$ }' ast-grep --pattern 'handleExecuteTransaction($_, $sender) { $$$ }'Length of output: 142
To locate and inspect the handler signatures directly, let’s search the codebase for their definitions and check the parameter lists:
🏁 Script executed:
#!/bin/bash # Locate and inspect handleValidateTransaction signature rg -Hn "handleValidateTransaction\s*\(" --include '*.ts' -A 2 # Locate and inspect handleExecuteTransaction signature rg -Hn "handleExecuteTransaction\s*\(" --include '*.ts' -A 2Length of output: 1010
Let’s broaden the search patterns and include context to locate both handlers across all
.tsfiles:
🏁 Script executed:
#!/bin/bash # Search for any occurrence of the handlers and show surrounding lines rg -n -g '*.ts' 'handleValidateTransaction' -C 3 rg -n -g '*.ts' 'function handleValidateTransaction' -C 3 rg -n -g '*.ts' 'handleExecuteTransaction' -C 3 rg -n -g '*.ts' 'function handleExecuteTransaction' -C 3Length of output: 10394
Confirmed: Handler methods accept the new
senderparameterBoth
ServerHandlers.handleValidateTransactionandServerHandlers.handleExecuteTransactionsignatures includesender: string, matching the propagated argument:
- src/libs/network/endpointHandlers.ts:
• handleValidateTransaction(tx: Transaction, sender: string)
• handleExecuteTransaction(validatedData: ValidityData, sender: string)LGTM – the change consistently propagates identity through the pipeline.
src/libs/blockchain/types/confirmation.ts (2)
13-13: Good addition: Import for unified signature algorithm type.The
SigningAlgorithmimport supports the transition to algorithm-aware signatures.
19-22:Details
✅ Verification successful
Verify compatibility: Significant signature type change.
The signature property has been changed from a raw buffer to a structured object with algorithm metadata. This aligns with the unified cryptographic refactor but is a breaking change.
Verify that all usages of
Confirmation.signaturehave been updated to handle the new structure:
🏁 Script executed:
#!/bin/bash # Description: Find all usages of Confirmation signature property to ensure compatibility # Search for references to Confirmation signature property rg -A 3 -B 1 "\.signature" --type ts # Search for Confirmation class instantiation and usage ast-grep --pattern 'new Confirmation()' # Search for signature assignments rg -A 2 "signature\s*=" --type tsLength of output: 32203
Confirmation.signature usage verified
All instantiations of Confirmation.signature have been updated to the new
{ type, data }shape:
- src/libs/blockchain/transaction.ts (line 152):
const confirmation = new Confirmation() confirmation.signature = { type: getSharedState.signingAlgorithm, data: uint8ArrayToHex(signature.signature), }No other references to Confirmation.signature were found.
src/libs/blockchain/chain.ts (3)
536-538: Good refactor: Dynamic signing algorithm for transaction recipient.Replacing the hardcoded
"ed25519"withgetSharedState.signingAlgorithmsupports multiple cryptographic algorithms.
540-542: Good refactor: Dynamic signing algorithm for transaction sender.Consistent use of dynamic signing algorithm from shared state.
544-547:Details
✅ Verification successful
Verify initialization: Dynamic signature algorithm and format change.
The changes align with the unified cryptographic refactor:
- Dynamic signing algorithm from shared state
- Signature data format changed from
Uint8Arrayto string"0x0"Verify that
getSharedState.signingAlgorithmis properly initialized before genesis block creation:
🏁 Script executed:
#!/bin/bash # Description: Verify shared state initialization order and signature algorithm setup # Search for signingAlgorithm initialization in shared state rg -A 5 -B 5 "signingAlgorithm.*=" --type ts # Search for genesis block generation calls to understand initialization order rg -A 3 -B 3 "generateGenesisBlock" --type ts # Check if there are any default values or initialization guards ast-grep --pattern 'signingAlgorithm: $_'Length of output: 2420
All good –
signingAlgorithmhas a default and is initialized before genesis creation.
- In
src/utilities/sharedState.ts,signingAlgorithmis assigned"ed25519"by default, sogetSharedState.signingAlgorithmwill always be defined whengenerateGenesisBlockruns.src/libs/network/manageNodeCall.ts (1)
148-149: Blind cast may hide runtime type errors
keypair.publicKeyshould already be aUint8Array; if future refactors turn it into a hex string, thisas Uint8Arraywill compile yet explode at run-time.-response.response = uint8ArrayToHex(getSharedState.keypair.publicKey as Uint8Array) +const pk = getSharedState.keypair.publicKey +response.response = ArrayBuffer.isView(pk) + ? uint8ArrayToHex(pk as Uint8Array) + : pk // assume already hexAdds a tiny guard without cost.
src/libs/abstraction/web2/twitter.ts (2)
7-7: LGTM: Consistent import addition for unified crypto refactor.The import of
SigningAlgorithmaligns with the broader migration to algorithm-aware cryptographic operations.
139-139: LGTM: Proper return type update for unified signature handling.The change from
publicKey: stringtotype: SigningAlgorithmis consistent with the refactoring to use structured signature metadata instead of raw public keys.src/libs/blockchain/mempool.ts (1)
23-23: LGTM: Proper imports for unified crypto refactor.The addition of
ISignature,SigningAlgorithmtypes anducryptomodule aligns with the migration to asynchronous cryptographic operations.Also applies to: 28-28
src/libs/abstraction/web2/parsers.ts (2)
1-1: LGTM: Consistent import for algorithm-aware parsing.The SigningAlgorithm import supports the transition to structured signature metadata.
57-57: LGTM: Abstract method signature updated consistently.The return type change ensures all implementing classes follow the new algorithm-aware signature structure.
src/index.ts (3)
36-36: LGTM: Proper imports for unified crypto system.The addition of crypto utilities from the demosdk supports the transition to the unified cryptographic API.
241-255: LGTM: Proper public key handling with algorithm-aware file naming.The transition to hex-based public key handling and algorithm-prefixed file naming supports multi-algorithm cryptography effectively. The use of
uint8ArrayToHexand inclusion of the signing algorithm in the filename is a good practice for the unified crypto system.
228-255:Details
✅ Verification successful
Verify the unified crypto initialization sequence.
Ensure that the crypto initialization properly integrates with the existing identity system and that the shared state is correctly populated.
Run this script to verify the initialization dependencies:
🏁 Script executed:
#!/bin/bash # Verify shared state and crypto initialization integration echo "Checking shared state keypair usage..." rg -A 5 -B 5 "getSharedState\.keypair" echo "Checking signing algorithm configuration..." rg -A 3 -B 3 "signingAlgorithm" echo "Checking public key hex usage..." rg -A 3 -B 3 "publicKeyHex"Length of output: 35265
🏁 Script executed:
#!/bin/bash # Display SharedState class definition to inspect identity & keypair properties sed -n '1,200p' src/utilities/sharedState.tsLength of output: 6538
🏁 Script executed:
#!/bin/bash # Inspect the Identity class to verify its ed25519 properties and initialization sed -n '1,200p' src/libs/identity.tsLength of output: 100
🏁 Script executed:
#!/bin/bash # Locate the Identity implementation to verify ed25519 fields rg --files-with-matches "class Identity" -n .Length of output: 137
🏁 Script executed:
#!/bin/bash # Inspect the Identity class to verify ed25519 properties and initialization sed -n '1,200p' src/libs/identity/identity.tsLength of output: 3344
🏁 Script executed:
#!/bin/bash # Show ucrypto import and context in src/index.ts sed -n '1,100p' src/index.ts | sed -n '1,200p' rg --context 3 "ucrypto" -n src/index.tsLength of output: 4542
Unified crypto initialization sequence is correct
The call order in src/index.ts —
getSharedState.identity.ensureIdentity()loads or generates the ed25519 key,ucrypto.generateAllIdentities(...)seeds the unified-crypto layer with that private key,getSharedState.keypair = await ucrypto.getIdentity(signingAlgorithm)populates the shared‐state keypair,- subsequent calls to
getSharedState.publicKeyHexreturn the expected value.No gaps were found in how
identity,keypair,signingAlgorithmandpublicKeyHexare initialized or used across the codebase.src/libs/abstraction/web2/github.ts (1)
40-42: Consistent return type for the unified crypto refactor.The updated return type correctly replaces
publicKeywithtypefield, aligning with the broader migration to the unified crypto module's signature format.src/features/web2/handleWeb2.ts (1)
80-93: Proper migration to async cryptographic signing.The conversion to
ucrypto.signwith explicit algorithm specification and hex encoding is correctly implemented, following the unified crypto module pattern.src/libs/abstraction/index.ts (1)
37-61: Well-structured async signature verification with proper error handling.The migration to
ucrypto.verifyis correctly implemented with:
- Proper destructuring of the new signature format
- Correct conversion of hex strings to Uint8Arrays
- Nested try-catch for specific error handling of verification failures
- Clear success/failure messages
src/libs/network/manageHelloPeer.ts (3)
15-18: Correct migration to structured signature format.The signature field properly follows the new unified crypto format with explicit algorithm type specification.
37-37: Simplified and consistent identity comparison.Good simplification using
getSharedState.publicKeyHexdirectly, aligning with the pattern used in other modules.
55-62: Properly implemented async signature verification.The migration to
ucrypto.verifycorrectly:
- Uses the algorithm type from the signature object
- Converts all inputs to Uint8Arrays
- Handles the async nature of the verification
src/libs/consensus/v2/routines/manageProposeBlockHash.ts (1)
8-8: Import looks OK – but double-check tree-shaking impact.
hexToUint8Arrayanducryptoare correctly imported.
No action required unless bundle size matters.src/libs/consensus/v2/routines/createBlock.ts (1)
41-44:Details
❓ Verification inconclusive
Verify the
ucrypto.signcall signature.Other calls to
ucrypto.verifyuse an object, but heresignis invoked
with positional parameters. Ensure this matches the actual API;
otherwise the returned value might beundefinedor contain unexpected
fields.
🏁 Script executed:
#!/bin/bash # Locate every ucrypto.sign invocation to confirm the call shape rg --line-number "ucrypto\.sign" -A1 -B1Length of output: 3710
🏁 Script executed:
#!/bin/bash # Show import and nearby context of ucrypto usage in createBlock.ts file="src/libs/consensus/v2/routines/createBlock.ts" echo "---- Imports in $file ----" sed -n '1,30p' "$file" echo echo "---- ucrypto.sign usage around line 41 ----" sed -n '30,60p' "$file" | sed -n '1,30p'Length of output: 2918
🏁 Script executed:
#!/bin/bash # Fetch package.json from unpkg to find the types entry echo "---- package.json types field ----" curl -s https://unpkg.com/@kynesyslabs/demosdk/encryption/package.json | grep '"types"' -A1 # Extract the types file path typesFile=$(curl -s https://unpkg.com/@kynesyslabs/demosdk/encryption/package.json \ | grep '"types"' \ | sed -E 's/.*"types": "([^"]+)".*/\1/') echo "Types file located at: $typesFile" # Fetch the TypeScript definitions and show the signature of ucrypto.sign echo "---- ucrypto.sign signature ----" curl -s https://unpkg.com/@kynesyslabs/demosdk/encryption/$typesFile \ | sed -n '/sign\(/,/^}/p'Length of output: 586
Confirm
ucrypto.signcall signaturePlease verify that
ucrypto.signactually accepts positional parameters—i.e.(algorithm, data)—rather than a single options object. All current calls (including increateBlock.ts:41) use the positional form, but if the published API expects something likeucrypto.sign({ algorithm, data }), these invocations will returnundefinedor an unexpected shape.Files to inspect:
- src/libs/consensus/v2/routines/createBlock.ts:41
- src/features/web2/handleWeb2.ts:80
- src/features/web2/Web2RequestManager.ts:86
- src/libs/peer/Peer.ts:189, 238
- src/libs/peer/PeerManager.ts:296
- src/libs/utils/demostdlib/deriveMempoolOperation.ts:193
- src/libs/blockchain/transaction.ts:89, 156
- src/libs/network/endpointHandlers.ts:151
- src/libs/blockchain/routines/validateTransaction.ts:118, 162, 188, 219
If the API signature is object‐based, update these calls accordingly to ensure the returned value includes the expected
.signatureproperty.src/libs/peer/PeerManager.ts (2)
159-160: Use strict equality on hex strings is fine – just ensure canonical form.Comparing
identitywithpublicKeyHexpresumes both are lowercase /
no 0x prefix. If not normalised, equality might fail.
310-314: Good move to structured signature object.The request now carries
{ type, data }, which future-proofs multi-algo
support. Looks solid.src/libs/consensus/v2/routines/broadcastBlockHash.ts (3)
7-7: LGTM: Import changes align with unified crypto refactor.The import of
ucryptoandhexToUint8Arrayfrom@kynesyslabs/demosdk/encryptionis consistent with the unified cryptographic operations introduced across the codebase.
16-16: LGTM: Simplified public key access.The change from
getSharedState.identity.ed25519.publicKey.toString("hex")togetSharedState.publicKeyHexsimplifies the public key access and aligns with the unified public key handling approach.
62-67:Details
✅ Verification successful
Verify the message encoding consistency.
The signature verification now uses
new TextEncoder().encode(block.hash)instead of the raw block hash string. Ensure this encoding is consistent with how signatures were originally created.
🏁 Script executed:
#!/bin/bash # Description: Check how block hashes are signed in the codebase to ensure consistency # Search for block hash signing patterns rg -A 5 -B 5 "sign.*block\.hash|block\.hash.*sign" --type tsLength of output: 62
I wasn’t able to locate any existing block-hash signing logic. Let’s search for where signatures are created to confirm the encoding used:
🏁 Script executed:
#!/bin/bash # Find all ucrypto method usages rg "ucrypto\." --type ts -n # Find all .sign( calls rg "\.sign\(" --type ts -n # Find all signature fields in objects rg "signature" --type ts -n -A3 -B3Length of output: 70913
Let’s confirm how the block hash is signed in createBlock.ts and see the surrounding context in broadcastBlockHash.ts:
#!/bin/bash # Show how the blockSignature is created rg -n "const blockSignature" -A3 -B3 src/libs/consensus/v2/routines/createBlock.ts # Show verify call context in broadcastBlockHash.ts sed -n '1,100p' src/libs/consensus/v2/routines/broadcastBlockHash.ts
Let’s inspect how the block hash is signed in createBlock.ts and view the full verify context in broadcastBlockHash.ts:
🏁 Script executed:
#!/bin/bash # Show how the blockSignature is created in createBlock.ts rg -n "ucrypto\.sign" -A3 -B3 src/libs/consensus/v2/routines/createBlock.ts # Show the surrounding context for verify in broadcastBlockHash.ts sed -n '1,80p' src/libs/consensus/v2/routines/broadcastBlockHash.tsLength of output: 3931
Encoding Consistency Verified
Both signing and verification use
new TextEncoder().encode(block.hash):
- In
src/libs/consensus/v2/routines/createBlock.ts(lines 41–44), the block signature is created with:const blockSignature = await ucrypto.sign( getSharedState.signingAlgorithm, new TextEncoder().encode(block.hash), )- In
src/libs/consensus/v2/routines/broadcastBlockHash.ts(lines 62–67), incoming signatures are verified with the same encoding:const isValid = await ucrypto.verify({ algorithm: getSharedState.signingAlgorithm, message: new TextEncoder().encode(block.hash), signature: hexToUint8Array(signature), publicKey: hexToUint8Array(identity), })No changes required.
src/libs/network/routines/transactions/handleIdentityRequest.ts (2)
37-42: LGTM: PQC identity support added correctly.The new
pqc_identity_assigncase properly delegates toIdentityManager.verifyPqcPayloadwith the correct payload type and sender parameter. The comment clearly explains the sender parameter semantics.
24-27:Details
✅ Verification successful
Breaking change: Verify all function callers are updated.
The function signature changed from accepting
IdentityPayloaddirectly to accepting a fullTransactionobject. This is a breaking change that requires all callers to be updated.
🏁 Script executed:
#!/bin/bash # Description: Find all callers of handleIdentityRequest to ensure they're updated for the new signature # Search for function calls to handleIdentityRequest rg -A 3 -B 3 "handleIdentityRequest\(" --type tsLength of output: 1228
No breaking change detected: all callers updated
A search for
handleIdentityRequest(shows a single caller insrc/libs/network/endpointHandlers.tsalready passing(tx, sender). No other usages were found, so no action is needed.src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
133-138:Details
❓ Verification inconclusive
Verify message encoding consistency for PQC signatures.
The verification uses
new TextEncoder().encode(payload.address)as the message. Ensure this encoding is consistent with how PQC signatures are created elsewhere in the codebase.
🏁 Script executed:
#!/bin/bash # Description: Check how PQC signatures are created to ensure message encoding consistency # Search for PQC signing patterns rg -A 5 -B 5 "pqc.*sign|sign.*pqc" --type tsLength of output: 2875
🏁 Script executed:
#!/bin/bash # Retrieve the verifyPqcPayload implementation to inspect how the message is encoded rg -A20 -B5 "verifyPqcPayload" --type ts src/libs/blockchain/gcr/gcr_routines/identityManager.tsLength of output: 1089
🏁 Script executed:
#!/bin/bash # List GCR routine files to locate potential signing methods ls -1 src/libs/blockchain/gcr/gcr_routines # Search for any ucrypto.sign usages across the repo rg -R "ucrypto\.sign" --type ts # Locate the PqcIdentityAssignPayload type definition rg -R "PqcIdentityAssignPayload" --type tsLength of output: 1245
🏁 Script executed:
#!/bin/bash # Correct search for ucrypto.sign usages rg "ucrypto\.sign" --type ts # Search for any general .sign( invocations in TS files rg "\.sign\(" --type ts | grep -v "verify" # Locate PqcIdentityAssignPayload definition rg "type PqcIdentityAssignPayload" -A5 --type tsLength of output: 3618
Ensure consistent message encoding for PQC identity‐assign signatures
Before merging, confirm that the way you encode
payload.addresswhen signing matches how you verify it. InverifyPqcPayload(src/libs/blockchain/gcr/gcr_routines/identityManager.ts lines 133–138), you currently do:message: new TextEncoder().encode(payload.address)If your PQC‐assign payloads were signed over the raw key bytes (e.g. via
hexToUint8Array(payload.address)), this will fail at verify time. Please:
- Verify how the client generates each
payload.signaturefor PQC identity assign:
• Are you signing the UTF-8 bytes of the hex string?
• Or the raw public‐key bytes decoded from hex?- If it’s the latter, update the verifier to:
message: hexToUint8Array(payload.address)- Otherwise, ensure your signing logic uses
new TextEncoder().encode(payload.address).src/model/entities/types/IdentityTypes.ts (1)
3-12: LGTM: Well-structured XM identity interface.The
SavedXmIdentityinterface provides a comprehensive structure for XM identity data with clear comments explaining the signed message semantics. The note about not storing the message but deriving it from the transaction is particularly helpful.src/libs/peer/Peer.ts (1)
40-44: Null-safe access to local public key
getSharedState.keypairmay be undefined during early bootstrap or in test environments.
Guard to avoidCannot read property 'publicKey' of undefined.-return this.identity === - uint8ArrayToHex(getSharedState.keypair.publicKey as Uint8Array) +return ( + getSharedState.keypair && + this.identity === + uint8ArrayToHex(getSharedState.keypair.publicKey as Uint8Array) +)src/libs/network/manageConsensusRoutines.ts (1)
76-78: EnsurepublicKeyHexexists in shared stateIf
publicKeyHexis undefined the shard-membership test always fails, leading to false 400 responses.
Double-check initialization order or fall back to computing it fromkeypair.publicKey.src/libs/consensus/v2/types/secretaryManager.ts (1)
538-539:Details
✅ Verification successful
Double-check the updated RPC payload shape
releaseWaitingMembersnow sends only[timestamp, phase].
Please confirm that everygreenlighthandler has been refactored accordingly; otherwise calls will break with400 / 500responses.
🏁 Script executed:
#!/bin/bash # Find all greenlight handlers and list their parameter count rg --context 2 $'function\\s+greenlight' | head -n 40Length of output: 53
Let's broaden the search to locate any references to
greenlightacross the codebase:
🏁 Script executed:
#!/bin/bash # Search for any occurrences of 'greenlight' to find handler definitions rg --context 2 'greenlight' | head -n 60Length of output: 4029
Let’s inspect the signatures and handler to ensure they match the new
[timestamp, phase]payload:#!/bin/bash # Show releaseWaitingMembers definition rg -A3 -B3 'releaseWaitingMembers' src/libs/consensus/v2/types/secretaryManager.ts # Show receiveGreenLight signature rg -A3 -B3 'receiveGreenLight' src/libs/consensus/v2/types/secretaryManager.ts # Show the greenlight RPC handler rg -A5 -B5 'case "greenlight":' src/libs/network/manageConsensusRoutines.ts
Let’s dump the relevant definitions and handler to verify their parameter shapes:
🏁 Script executed:
#!/bin/bash # Show releaseWaitingMembers definition rg -A3 -B3 'releaseWaitingMembers' src/libs/consensus/v2/types/secretaryManager.ts # Show receiveGreenLight signature rg -A3 -B3 'receiveGreenLight' src/libs/consensus/v2/types/secretaryManager.ts # Show the greenlight RPC handler rg -A5 -B5 'case "greenlight":' src/libs/network/manageConsensusRoutines.tsLength of output: 1945
Greenlight RPC payload shape is correct
The
greenlighthandler inmanageConsensusRoutines.tsunpacks exactly two params—[timestamp, validatorPhase]—and forwards them toSecretaryManager.receiveGreenLight(timestamp, validatorPhase), whose signature matches. No othergreenlighthandlers exist, so no further refactoring is needed.src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
48-68:SavedXmIdentityextended object – butpublicKeymay beundefined
publicKeyis pushed witheditOperation.data.publicKey || "".
If the caller forgetspublicKey, the empty string is persisted and later
signature verification will inevitably fail.Either make the field mandatory in the validation above, or omit it when not
provided to avoid writing meaningless data.- !signature || - !timestamp || - !signedData + !signature || + !timestamp || + !signedData || + !publicKeyor
publicKey: editOperation.data.publicKey || "",-> remove the line and adjust the type.
Choose one consistent path.src/libs/blockchain/transaction.ts (2)
152-163: Type mismatch when buildingConfirmation
Confirmation.data.validatorexpects aforge.pki.ed25519.BinaryBufferbut an
Uint8Arrayis assigned. Down-stream code that serialises / deserialises the
confirmation may break.Convert the key:
-confirmation.data.validator = getSharedState.keypair.publicKey as Uint8Array +confirmation.data.validator = forge.util.binary.raw.encode( + Buffer.from(getSharedState.keypair.publicKey) +)(or update the type definition if the project is switching away from Forge).
256-262: Double hashing risk – message format not fixedThe main signature is verified on
TextEncoder().encode(tx.hash).
If the original signer instead signed the raw content (pre-hash) this
verification will always fail. Ensure signing and verification agree on:
- raw JSON
- canonicalised JSON
- or the hash string
Document the convention and enforce it in
sign().
Refactor signature verification & signing, node identity and other cryptography modules to work with the unified crypto module.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores