feat: support for getting blocks from Handshake peer

[agaffney]

Summary by CodeRabbit

• New Features

  • Added support for inventory-style data requests and block payloads to the handshake protocol.
  • Enabled retrieving blocks from peers with request/response handling and error paths.

• Tests

  • Added tests to validate encoding/decoding of data request and block messages.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds inventory and block message support to the handshake protocol. It introduces InvItem (Type uint32, Hash [32]byte) and inventory type constants, MsgGetData (Inventory []InvItem) and MsgBlock (Block *handshake.Block) with Encode/Decode implementations, and wires those message types into the protocol decoder. MsgGetData encodes a varint length and 36-byte items; MsgBlock decodes a block from a reader (Encode currently stubbed). Peer.GetBlock(hash) was added to send a getdata request and return a received MsgBlock. A unit test verifies MsgGetData encode/decode round-trip.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Encode/Decode logic for InvItem, MsgGetData, and MsgBlock (varint handling, per-item 36-byte layout).
• Peer.GetBlock: message send/receive flow and error paths.
• Integration into decodeMessage switch to ensure correct message construction.
• Review the new test for coverage and correctness.

Possibly related PRs

• feat: initial handshake network peer support #422 — Adds the peer send/receive and handshake messaging infrastructure that Peer.GetBlock and the new messages build on.

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 directly describes the main change: adding support for getting blocks from Handshake peers, which aligns with the PR's new GetBlock method and supporting infrastructure.

✨ 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 feat/handshake-get-data

---

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: 2

🧹 Nitpick comments (2)

internal/handshake/protocol/peer.go (1)

242-265: GetBlock flow is consistent; consider future-proofing for interleaved messages

This mirrors GetHeaders/GetProof nicely: you build a single InvTypeBlock entry, send MessageGetData, then expect a *MsgBlock and return its Block. The implementation looks correct as long as the peer responds directly with a block message.

If, in future, peers can legally send other (supported) messages before the block response, you may want to loop on receiveMessage() until you see a *MsgBlock or hit a timeout, instead of failing on the first non‑MsgBlock message.

internal/handshake/protocol/messages.go (1)

460-476: MsgBlock.Decode looks fine; consider guarding the Encode stub

MsgBlock.Decode delegating to handshake.NewBlockFromReader(bytes.NewReader(data)) is straightforward and matches the rest of the API.

MsgBlock.Encode currently returns an empty slice with a comment explaining that block encoding isn’t implemented. That’s okay as long as MsgBlock is only ever used on the receive side, but if a caller accidentally tries to send a MessageBlock, it will silently produce an invalid zero‑length payload.

Optional: change Encode to fail fast (e.g., panic("MsgBlock.Encode not implemented") or log/metrics) so misuse is obvious during development.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5480036 and 3b220f3.

📒 Files selected for processing (3)

• internal/handshake/protocol/messages.go (5 hunks)
• internal/handshake/protocol/messages_test.go (1 hunks)
• internal/handshake/protocol/peer.go (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

internal/handshake/protocol/peer.go (2)

internal/handshake/block.go (1)

• Block (19-22)

internal/handshake/protocol/messages.go (5)

• MsgGetData (353-355)
• InvItem (534-537)
• InvTypeBlock (527-527)
• MessageGetData (28-28)
• MsgBlock (460-462)

internal/handshake/protocol/messages.go (1)

internal/handshake/block.go (2)

• Block (19-22)
• NewBlockFromReader (24-37)

internal/handshake/protocol/messages_test.go (1)

internal/handshake/protocol/messages.go (4)

• Message (51-54)
• MsgGetData (353-355)
• InvItem (534-537)
• InvTypeBlock (527-527)

🔇 Additional comments (2)

internal/handshake/protocol/messages.go (2)

20-35: New GetData/Block message wiring looks consistent; verify IDs against Handshake spec

Adding MessageGetData and MessageBlock and wiring them into decodeMessage via &MsgGetData{} / &MsgBlock{} follows the existing pattern for other message types and looks correct from a code perspective.

Because these numeric IDs are part of the on‑wire Handshake protocol, it’s worth double‑checking them against the upstream spec/hsd implementation to avoid subtle interoperability issues.

Also applies to: 73-103

---

525-553: Fix InvItem.Decode hash assignment; avoid invalid slice-to-array conversion

InvItem.Decode validates the 36‑byte length correctly, but the hash assignment uses the same invalid pattern as in the tests:

i.Hash = [32]byte(data[4:])

Converting a []byte to a [32]byte like this is not allowed in Go and will fail to compile. Use an explicit copy into the fixed-size array:

 func (i *InvItem) Decode(data []byte) error {
 	if len(data) != 36 {
 		return errors.New("invalid payload length")
 	}
-	i.Type = binary.LittleEndian.Uint32(data[0:4])
-	i.Hash = [32]byte(data[4:])
+	i.Type = binary.LittleEndian.Uint32(data[0:4])
+	copy(i.Hash[:], data[4:])
 	return nil
 }

Same pattern appears in other decode paths in this file; it may be worth standardizing on a helper for 32‑byte hash copies.

⛔ Skipped due to learnings

Learnt from: agaffney
Repo: blinklabs-io/cdnsd PR: 424
File: internal/handshake/protocol/messages_test.go:132-136
Timestamp: 2025-11-12T22:02:04.720Z
Learning: In Go 1.20 and later, slices can be converted directly to arrays using the syntax `[N]T(slice)` where N is the array size and T is the element type. For example, `[32]byte(mySlice)` is valid and will copy the slice elements into a new array. This conversion panics if the slice length is less than the array size.