fix: use correct read uvarint function when decoding Handshake covenants

[agaffney]

Summary by cubic

Fixed Handshake covenant decoding by using ReadUvarintReader for varints instead of binary.ReadUvarint with a ByteReader cast. Prevents panics on non-ByteReader inputs and correctly parses item counts and lengths.

^{Written for commit 53830a8. Summary will update automatically on new commits.}

Summary by CodeRabbit

• Refactor
  • Internal improvements to code reliability and maintainability. No user-facing changes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The Decode method in internal/handshake/covenant.go is refactored to replace direct calls to binary.ReadUvarint() with a new ReadUvarintReader() helper function. Two instances of reading varint values (itemCount and itemLength) are updated to use the new helper instead of performing type assertion to io.ByteReader. The control flow for looping through items and reading bytes remains unchanged, and error handling is preserved.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

• Verify that ReadUvarintReader() provides functionally equivalent behavior to the type-asserted binary.ReadUvarint() approach
• Confirm error propagation from the helper function matches the original implementation
• Check that the change doesn't affect the reading sequence or item parsing logic

Possibly related PRs

• fix: use correct varint decoding for Handshake #439: Replaces binary.ReadUvarint with ReadUvarintReader in internal/handshake decoders, applying the same varint reading refactoring pattern across handshake decoding methods.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: replacing an incorrect io.ByteReader type assertion with a proper helper function (ReadUvarintReader) in the Decode method.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/handshake-read-uvarint

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

internal/handshake/covenant.go (1)

46-56: Consider bounds-checking itemCount and itemLength to avoid huge allocations

Because ReadUvarintReader can return arbitrarily large uint64 values from untrusted input, for range itemCount and make([]byte, itemLength) could be abused to force very large loops/allocations. If this code runs on untrusted network data, consider enforcing protocol-level limits on itemCount and itemLength (e.g., max items per covenant, max item size) and returning an error when exceeded.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f97e95 and 53830a8.

📒 Files selected for processing (1)

• internal/handshake/covenant.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

internal/handshake/covenant.go (1)

internal/handshake/varint.go (1)

• ReadUvarintReader (29-70)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: cubic · AI code reviewer
• GitHub Check: Analyze (go)

🔇 Additional comments (1)

internal/handshake/covenant.go (1)

42-52: Correct helper used for Handshake varint decoding

Switching both itemCount and itemLength to ReadUvarintReader(r) aligns covenant decoding with the custom Handshake varint format implemented in internal/handshake/varint.go and avoids relying on the reader implementing io.Reader+io.ByteReader. Error propagation and control flow remain unchanged, so this looks like a safe and targeted correctness fix.

[cubic-dev-ai]

No issues found across 1 file