chore: minor changes from audit tool

[tac0turtle]

Overview

Testing out an audit tool and it found these items.

Summary by CodeRabbit

• Chores

  • Updated mapstructure dependency to latest version.

• Tests

  • Added fuzzing tests for serialization of core data structures.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: de452420-b531-4a9f-ba6a-604ff5a31d38

📥 Commits

Reviewing files that changed from the base of the PR and between 5fa641d and 46e8b30.

⛔ Files ignored due to path filters (2)

• apps/grpc/go.sum is excluded by !**/*.sum
• apps/testapp/go.sum is excluded by !**/*.sum

📒 Files selected for processing (2)

• apps/grpc/go.mod
• apps/testapp/go.mod

💤 Files with no reviewable changes (2)

• apps/testapp/go.mod
• apps/grpc/go.mod

---

📝 Walkthrough

Walkthrough

This pull request upgrades the mapstructure dependency from mitchellh/mapstructure v1.5.0 to go-viper/mapstructure/v2 v2.4.0 across the codebase, updates the import statement in the config package, optimizes proposer address comparison to use byte slices instead of strings, and introduces fuzz tests for serialization functions.

Changes

Cohort / File(s)	Summary
Dependency Migration go.mod, pkg/config/config.go, apps/grpc/go.mod, apps/testapp/go.mod	Upgraded mapstructure dependency from mitchellh/mapstructure v1.5.0 to go-viper/mapstructure/v2 v2.4.0; updated import statement to reference the new package location; removed old dependency entries from go.mod files.
Byte Comparison Optimization block/internal/syncing/assert.go	Replaced string-based proposer address comparison with bytes.Equal for raw byte slice comparison; added "bytes" import.
Fuzz Testing types/serialization_fuzz_test.go	Added comprehensive fuzz tests covering binary marshaling/unmarshaling of Header, Data, SignedHeader, SignedData, and DAEnvelope; included fuzzing for protobuf conversion paths (ToProto/FromProto).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

• tzdybal
• randygrok

Poem

🐰 Hop along, dear code, to versions new!
From mitchellh's path to viper's view,
Bytes compare where strings once stood,
And fuzzing tests ensure all's good! ✨

🚥 Pre-merge checks | ❌ 3

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description is incomplete and lacks essential details about what the audit tool found and why these specific changes address those findings.	Expand the description to explain what the audit tool identified, why these changes were made, and provide specific context about the byte comparison change and dependency migration.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'chore: minor changes from audit tool' is vague and generic, using non-descriptive terms like 'minor changes' that don't convey meaningful information about the specific changeset.	Replace the generic title with a specific description of the primary change, such as 'chore: migrate mapstructure dependency to go-viper/mapstructure/v2' to better reflect the main modifications.

_{✏️ 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
• Commit unit tests in branch marko/audit-bits

📝 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

• X
• Mastodon
• Reddit
• LinkedIn

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

Tip

Migrating from UI to YAML configuration.

Use the @coderabbitai configuration command in a PR comment to get a dump of all your UI settings in YAML format. You can then edit this YAML file and upload it to the root of your repository to configure CodeRabbit programmatically.

[github-actions]

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	⏩ skipped	✅ passed	✅ passed	Mar 16, 2026, 10:43 PM

[claude]

Claude finished @julienrbrt's task in 1m 48s —— View job

---

Code Review

• Gather context
• Read changed files
• Analyze code quality, concurrency, security, performance
• Post review feedback

---

Overall this is a clean, well-scoped PR. The reverts address the valid concerns raised by @julienrbrt. Here are my findings across each change:

---

block/internal/syncing/assert.go — Byte comparison fix ✅

Replacing string-based proposer address comparison with bytes.Equal is correct and idiomatic. Using string comparison on raw byte slices is fragile and can produce false negatives if the bytes contain null terminators or non-printable characters. This is a good fix.

---

types/serialization_fuzz_test.go — Fuzz tests ✅ with minor notes

Good addition overall. Fuzzing serialization paths is valuable for finding panics on malformed inputs. A few observations:

FuzzDAEnvelopeUnmarshalBinary naming mismatch (mirrors CodeRabbit's finding)

// types/serialization_fuzz_test.go:75-85
func FuzzDAEnvelopeUnmarshalBinary(f *testing.F) {
    f.Fuzz(func(t *testing.T, data []byte) {
        var sh SignedHeader  // operates on SignedHeader, not a DAEnvelope type
        if _, err := sh.UnmarshalDAEnvelope(data); err != nil {
            return
        }
        _, _ = sh.MarshalDAEnvelope(nil)
    })
}

The function is named FuzzDAEnvelopeUnmarshalBinary but operates entirely on SignedHeader. Rename to FuzzSignedHeaderDAEnvelope (or similar) and the variable sh to signedHeader to make intent clear. Fix this →

Sparse seed corpus for FuzzSignedHeader and FuzzSignedData

FuzzSignedHeaderUnmarshalBinary and FuzzSignedDataUnmarshalBinary only seed with empty/trivial bytes. Compare this to FuzzHeaderUnmarshalBinary and FuzzDataUnmarshalBinary which seed from valid marshaled instances. Adding a valid marshaled seed gives the fuzzer a better starting point for exploring realistic code paths:

func FuzzSignedHeaderUnmarshalBinary(f *testing.F) {
    // Consider seeding with a valid marshaled SignedHeader if constructable
    f.Add([]byte{})
    f.Add([]byte{0x0a, 0x00})
    ...
}

This is a minor improvement, not a blocker.

FuzzStateFromProto only tests one direction

The test exercises proto.Unmarshal → FromProto but not the ToProto → proto.Marshal round-trip. The other fuzz tests all verify both marshal and unmarshal. Consider adding a ToProto call on successfully-unmarshaled State to catch panics in the serialization path too.

---

go.mod / pkg/config/config.go — Dependency migration ✅

Migrating from mitchellh/mapstructure v1.5.0 to go-viper/mapstructure/v2 v2.4.0 is the right call. The go-viper fork is the actively maintained successor and is used by viper itself. The import path change in pkg/config/config.go:14 is correct.

No breaking changes observed — the DecoderConfig struct, ComposeDecodeHookFunc, and hook functions used here are all present in v2 with the same API.

---

Reverted syncer.go changes

The reverts correctly removed the closeOnce guard and the no-op change flagged by @julienrbrt. The existing Stop() guard (if s.cancel == nil { return nil } at line 231) is sufficient to prevent double-close, so the sync.Once was indeed unnecessary complexity.

---

Summary

Change	Assessment
assert.go: bytes.Equal	✅ Correct fix
serialization_fuzz_test.go	✅ Good addition, minor naming nit
go.mod / config.go: mapstructure v2	✅ Clean dependency upgrade
Reverted syncer.go changes	✅ Appropriate reverts

The only actionable item is renaming FuzzDAEnvelopeUnmarshalBinary / the sh variable, which is a clarity improvement. The PR is otherwise ready to merge.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.41%. Comparing base (6f1d539) to head (46e8b30).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3165      +/-   ##
==========================================
+ Coverage   60.24%   60.41%   +0.17%     
==========================================
  Files         115      115              
  Lines       11902    11902              
==========================================
+ Hits         7170     7191      +21     
+ Misses       3921     3903      -18     
+ Partials      811      808       -3     

Flag	Coverage Δ
combined	60.41% <100.00%> (+0.17%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[coderabbitai]

🧹 Nitpick comments (1)

types/serialization_fuzz_test.go (1)

75-85: Consider renaming for clarity.

The function FuzzDAEnvelopeUnmarshalBinary operates on SignedHeader type rather than a dedicated DAEnvelope type. While functionally correct (testing UnmarshalDAEnvelope and MarshalDAEnvelope methods), the variable name sh and type SignedHeader could be confusing to readers expecting a separate envelope type.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@types/serialization_fuzz_test.go` around lines 75 - 85, Rename the fuzz test
and local variable to reflect it operates on SignedHeader: update the test
function name FuzzDAEnvelopeUnmarshalBinary to something like
FuzzSignedHeaderUnmarshalDAEnvelope (or FuzzSignedHeaderDAEnvelope) and rename
the local variable sh to a clearer name such as signedHeader; keep calling
SignedHeader.UnmarshalDAEnvelope and SignedHeader.MarshalDAEnvelope as-is so the
test behavior is unchanged but the intent is clearer.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@types/serialization_fuzz_test.go`:
- Around line 75-85: Rename the fuzz test and local variable to reflect it
operates on SignedHeader: update the test function name
FuzzDAEnvelopeUnmarshalBinary to something like
FuzzSignedHeaderUnmarshalDAEnvelope (or FuzzSignedHeaderDAEnvelope) and rename
the local variable sh to a clearer name such as signedHeader; keep calling
SignedHeader.UnmarshalDAEnvelope and SignedHeader.MarshalDAEnvelope as-is so the
test behavior is unchanged but the intent is clearer.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a24d85d2-edf9-4954-be1d-3682cd38fa19

📥 Commits

Reviewing files that changed from the base of the PR and between eabde1a and 5fa641d.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (5)

• block/internal/syncing/assert.go
• block/internal/syncing/syncer.go
• go.mod
• pkg/config/config.go
• types/serialization_fuzz_test.go