Skip to content

Conversation

@sewqasreedas
Copy link
Contributor

  • fix the 256-bit Int.Unmarshal implementation so empty byte slices zero the value instead of discarding the receiver pointer
  • initialize the internal big.Int when needed, mirroring existing patterns elsewhere in the math package
  • surface an explicit error if a nil receiver is passed, avoiding silent corruption

@aljo242
Copy link
Contributor

aljo242 commented Nov 17, 2025

can you show why this is needed?

@sewqasreedas
Copy link
Contributor Author

@aljo242 added test TestIntUnmarshalEmpty that demonstrates the bug - when unmarshaling empty bytes, the old code left the value unchanged (12345), while the correct behavior is to reset to zero.

without the fix, the test fails:
--- FAIL: TestIntUnmarshalEmpty (0.00s)
int_test.go:709:
Error: Should be true
Messages: after unmarshaling empty bytes, Int should be zero, but got: 12345

If I had to briefly answer why this is needed: the old code does i = nil which only reassigns a local variable and doesn't actually modify the receiver. This violates the basic Go contract for pointer receiver methods - they must modify the object itself, not reassign the pointer. As a result, calling Unmarshal with empty bytes (which represents field not set in protobuf) leaves the Int with its old value instead of properly resetting to zero.

@codecov
Copy link

codecov bot commented Nov 18, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.47%. Comparing base (4c28078) to head (0906e8f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
math/int.go 66.66% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #25568      +/-   ##
==========================================
+ Coverage   70.29%   70.47%   +0.18%     
==========================================
  Files         821      826       +5     
  Lines       53686    54630     +944     
==========================================
+ Hits        37740    38502     +762     
- Misses      15946    16128     +182     
Files with missing lines Coverage Δ
math/int.go 88.27% <66.66%> (ø)

... and 6 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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