Skip to content

fix(decoders): guard against undefined fields on Bradbury testnet#144

Merged
cristiam86 merged 1 commit intogenlayerlabs:mainfrom
acastellana:fix/decode-transaction-bradbury-compat
Mar 16, 2026
Merged

fix(decoders): guard against undefined fields on Bradbury testnet#144
cristiam86 merged 1 commit intogenlayerlabs:mainfrom
acastellana:fix/decode-transaction-bradbury-compat

Conversation

@acastellana
Copy link
Contributor

@acastellana acastellana commented Mar 16, 2026

Problem

decodeTransaction crashes when called on transactions from Bradbury testnet with:

TypeError: Cannot read properties of undefined (reading 'toString')
    at decodeTransaction (dist/index.js:1015:55)

Root cause

Bradbury's consensusDataContract.getTransactionData returns a different struct than studionet/localnet:

Field studionet/localnet Bradbury
numOfInitialValidators ✅ present ❌ absent
equivalent initialRotations
readStateBlockRange sub-fields ✅ present may be absent
lastRound sub-fields ✅ present may be absent

decodeTransaction called .toString() unconditionally on all these bigint fields, crashing on undefined.

Fix

  1. Use optional chaining + ?? "0" fallback for all bigint field conversions
  2. Fall back to initialRotations when numOfInitialValidators is absent
  3. Update GenLayerRawTransaction type to mark both fields as optional
  4. Guard lastRound and readStateBlockRange sub-fields consistently

Tested against

  • Bradbury tx: 0xb3183d4fb2ab7dcafed42e592d7f8cc075d3dfe720c38dd2b45459830326c778
  • Contract deployed at: 0xd77F1Df3103AfB8715b715992b2DBaf8d5529134
  • Build passes: npm run build

Summary by CodeRabbit

  • Bug Fixes
    • Improved transaction data parsing to safely handle missing or undefined fields across multiple transaction properties.
    • Added support for alternate field names in transaction types for enhanced backward compatibility.

Bradbury's consensusDataContract.getTransactionData returns a different
struct than studionet/localnet — notably:
- numOfInitialValidators is absent; the equivalent field is initialRotations
- readStateBlockRange and lastRound sub-fields may be absent

Previously decodeTransaction called .toString() unconditionally on all
these bigint fields, causing:
  TypeError: Cannot read properties of undefined (reading 'toString')

Fixes:
- Use optional chaining + fallback '0' for all bigint field conversions
- Fall back to initialRotations when numOfInitialValidators is absent
- Update GenLayerRawTransaction type to mark both fields as optional
- Guard lastRound sub-fields similarly

Tested against: Bradbury tx 0xb3183d4fb2ab7dcafed42e592d7f8cc075d3dfe720c38dd2b45459830326c778
@coderabbitai
Copy link

coderabbitai bot commented Mar 16, 2026

📝 Walkthrough

Walkthrough

Enhanced transaction decoder safety by implementing optional chaining and nullish coalescing operators across scalar fields and nested properties. Added initialRotations as an alternative source for numOfInitialValidators in the transaction type, improving robustness against missing or alternate field structures.

Changes

Cohort / File(s) Summary
Transaction Type Definitions
src/types/transactions.ts
Made numOfInitialValidators optional and introduced initialRotations as a fallback field with Bradbury equivalence.
Transaction Decoder Implementation
src/transactions/decoders.ts
Applied optional chaining and nullish coalescing (?. and ?? "0") to scalar fields (txSlot, timestamps, rounds, validators). Added dual-source handling for numOfInitialValidators with fallback to initialRotations. Hardened access to nested fields in readStateBlockRange and lastRound.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • feat: expose encode and decode methods #102: Directly related modifications to the same decoder and type files that establish foundational optional chaining patterns and introduce the initialRotations field for backward compatibility.

Suggested reviewers

  • epsjunior
  • danielrc888

Poem

🐰 With gentle chains and defaults so keen,
The decoder now shields what's unseen.
Where fields may hide or alternate dwell,
Safe access ensures all works well! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: guarding against undefined fields on the Bradbury testnet in decoders.
Description check ✅ Passed The description provides a well-structured problem statement, root cause analysis, detailed fixes, and testing evidence, but lacks explicit sections matching the template structure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@acastellana acastellana requested a review from cristiam86 March 16, 2026 19:21
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/transactions/decoders.ts (1)

85-86: Remove unnecessary as any cast.

Since initialRotations is now properly typed as bigint | undefined in GenLayerRawTransaction (line 280), the as any cast is no longer needed and bypasses type safety.

♻️ Proposed fix
     // Bradbury uses `initialRotations`; older chains use `numOfInitialValidators`
-    numOfInitialValidators: (tx.numOfInitialValidators ?? (tx as any).initialRotations)?.toString() ?? "0",
+    numOfInitialValidators: (tx.numOfInitialValidators ?? tx.initialRotations)?.toString() ?? "0",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/transactions/decoders.ts` around lines 85 - 86, The line constructing
numOfInitialValidators uses an unnecessary type assertion (tx as any). Remove
the cast and rely on the typed property by changing the expression to use
tx.numOfInitialValidators ?? tx.initialRotations and then call toString() (or
fallback "0"); update the reference in the decoder where numOfInitialValidators
is assigned (the expression building numOfInitialValidators from tx) to omit the
as any cast and use the properly typed GenLayerRawTransaction.initialRotations
instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/transactions/decoders.ts`:
- Around line 85-86: The line constructing numOfInitialValidators uses an
unnecessary type assertion (tx as any). Remove the cast and rely on the typed
property by changing the expression to use tx.numOfInitialValidators ??
tx.initialRotations and then call toString() (or fallback "0"); update the
reference in the decoder where numOfInitialValidators is assigned (the
expression building numOfInitialValidators from tx) to omit the as any cast and
use the properly typed GenLayerRawTransaction.initialRotations instead.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c3d907f3-a6de-4698-99ed-b34a3c8d45a4

📥 Commits

Reviewing files that changed from the base of the PR and between d65446b and c54645c.

📒 Files selected for processing (2)
  • src/transactions/decoders.ts
  • src/types/transactions.ts

@cristiam86 cristiam86 merged commit 328ed5c into genlayerlabs:main Mar 16, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants