Skip to content

Fix VerifyEvents to handle amounts exceeding int64 range#5932

Merged
karthikiyer56 merged 3 commits intomainfrom
fix/verify-events-big-int
Apr 7, 2026
Merged

Fix VerifyEvents to handle amounts exceeding int64 range#5932
karthikiyer56 merged 3 commits intomainfrom
fix/verify-events-big-int

Conversation

@karthikiyer56
Copy link
Copy Markdown
Contributor

@karthikiyer56 karthikiyer56 commented Apr 6, 2026

This PR fixes 2 issues:

1/
Replace int64 balance tracking with *big.Int in findBalanceDeltasFromEvents and all related functions. This fixes the unrecovered panic in MustParseInt64Raw when processing SAC events with i128 amounts that exceed int64 range (e.g., cumulative mints producing balances >2^63-1).

findBalanceDeltasFromEvents now returns an error instead of panicking on unparseable amounts. No exported API signatures changed.

Fixes #5929


2/
Fix NewMaxFee() which was truncating the int64 fee-bump fee to uint32, silently corrupting any fee-bump fee above ~429.5 XLM.
Return type changed from uint32 to int64 (breaking change)

Fixes #5931

Replace int64 balance tracking with *big.Int in findBalanceDeltasFromEvents
and all related functions. This fixes the unrecovered panic in
MustParseInt64Raw when processing SAC events with i128 amounts that
exceed int64 range (e.g., cumulative mints producing balances >2^63-1).

findBalanceDeltasFromEvents now returns an error instead of panicking
on unparseable amounts. No exported API signatures changed.

Fixes #5929

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 6, 2026 21:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the token transfer verification/reconciliation logic to correctly handle SAC event amounts that exceed the int64 range by switching internal balance-delta tracking from int64 to *big.Int, and by returning errors instead of panicking on invalid amounts. This addresses the VerifyEvents failure described in issue #5929.

Changes:

  • Replace int64 delta maps with map[balanceKey]*big.Int across event/change delta aggregation and reconciliation paths.
  • Add safe parsing of event amount strings into *big.Int, returning errors for invalid amounts instead of panicking.
  • Add unit tests covering large (i128-range) amounts, invalid amounts, and updateBalanceMap behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
processors/token_transfer/verify_events.go Switch delta tracking to *big.Int, add amount parsing with error returns, and compare maps using a big.Int comparer.
processors/token_transfer/verify_events_test.go New tests for large SAC amounts, invalid amounts, fee/clawback, and map update behavior.
processors/token_transfer/token_transfer_processor.go Update reconciliation logic to consume *big.Int delta maps and emit mint/burn events using big-int string amounts.
Comments suppressed due to low confidence (1)

processors/token_transfer/verify_events.go:167

  • findBalanceDeltasFromEvents calls event.GetAsset() before switching on the concrete event type. TokenTransferEvent.GetAsset() panics on unknown event types, so this can still crash VerifyEvents even though the switch default now returns an error. To avoid unrecovered panics, restructure to avoid calling GetAsset() until after the event type is known (or use type-specific getters to access the asset).
	for _, event := range events {
		if event.GetAsset() == nil { // needed check for custom token events which won't have an asset
			continue
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…parer

- Short-circuit updateBalanceMap when delta is zero to prevent inserting
  spurious zero-valued map entries (could cause false verify mismatches)
- Move bigIntComparer allocation outside the transaction loop in
  VerifyEvents since it's stateless and reusable
- Add test for zero-delta edge case

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@karthikiyer56 karthikiyer56 requested a review from a team April 7, 2026 05:14
FeeBumpTransaction.Fee is XDR Int64 but NewMaxFee() was casting it to
uint32, silently discarding the upper 32 bits. Any fee-bump fee above
~429.5 XLM (4,294,967,295 stroops) returned a wrong value.

Change return type from uint32 to int64 — this is a breaking change.

Fixes #5931

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@karthikiyer56 karthikiyer56 merged commit 475bbd9 into main Apr 7, 2026
11 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

3 participants