Skip to content

Commit 07f94a8

Browse files
committed
fix(payment): prevent Int64 overflow panic in settle-timestamp arithmetic
UpdateStreamRecord and TryResumeStreamRecord computed settle timestamps by calling Int.Int64() on an unbounded sdkmath.Int. When a stream account's pay-duration exceeded MaxInt64 seconds (~2.9e11 years) the call panicked with "Int64() out of bound". Because both functions run inside the storage EndBlocker with no recover(), the panic was deterministic across every validator and halted the chain. Fix: perform the full settle-timestamp expression in sdkmath.Int, then handle an out-of-range result by intent rather than by call site: - A user-initiated deposit (positive static-balance change, no rate change) that would overflow is rejected with ErrSettleTimestampOverflow — the depositor is over-funding past what we can represent and can deposit less. - Every other path saturates the settle timestamp to math.MaxInt64: forced EndBlocker updates (where an error would re-trigger the same halt via abci.go "should not happen"), rate decreases from object deletion or discontinue, and lazy re-pricing when the SP price falls. The settle timestamp is only a scheduling hint for the auto-settle queue and is recomputed on the next balance change or bucket touch, so capping it is loss-free and never blocks a legitimate object deletion. Closes MOCA-385
1 parent a018ade commit 07f94a8

4 files changed

Lines changed: 229 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
6666
- (app/upgrades) [#289](https://github.com/mocachain/moca/pull/289) Pin v2 feemarket `min_gas_price` to 20 gwei (moca's intended floor) so upgraded chains match genesis.
6767
- (evm) [#290](https://github.com/mocachain/moca/pull/290) Restore `CallEVM` error-context wrap (method + contract in error message), fix copy-pasted "evil token" comment in `erc721NonTransferable.go`, update stale geth v1.15→v1.16 comments, remove unreachable `AddBalance` blocks in distribution precompile, fix grammar in precompile Run() default cases, and drop dead test-helper expressions.
6868
- (evm) [#291](https://github.com/mocachain/moca/pull/291) Reject native value sent to precompiles to prevent a balance-reconciliation mint.
69+
- (x/payment) [MOCA-385] Prevent Int64 overflow panic in `UpdateStreamRecord` and `TryResumeStreamRecord`: settle-timestamp arithmetic is now performed in arbitrary-precision `sdkmath.Int` instead of native `int64`. Only a user-initiated deposit that would push the pay duration past `MaxInt64` is rejected (`ErrSettleTimestampOverflow`) — the depositor can simply deposit less. Every other path (rate decreases from object deletion/discontinue, lazy re-pricing when the SP price falls, and all forced EndBlocker updates) saturates the settle timestamp to `MaxInt64` instead, since it is only a scheduling hint that is recomputed on the next balance change or bucket touch. This keeps a legitimate object deletion from ever being rejected and prevents the EndBlocker from panicking (which would halt the chain).
6970
- (x/challenge) [#286](https://github.com/mocachain/moca/pull/286) Retire a challenge from the active set once it is attested, making attestation idempotent so duplicate submissions (e.g. redundant relayers or resubmissions by the in-turn submitter) are rejected instead of re-running heartbeat rewards and re-emitting attestation events.
7071
- (ci) [#65](https://github.com/mocachain/moca/pull/65) Resolve goreleaser CI failures for arm64 docker builds
7172
- (audit) [#63](https://github.com/mocachain/moca/pull/63) Apply audit fixes

x/payment/keeper/stream_record.go

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package keeper
22

33
import (
44
"fmt"
5+
"math"
56

67
sdkmath "cosmossdk.io/math"
78
"cosmossdk.io/store/prefix"
@@ -214,7 +215,40 @@ func (k Keeper) UpdateStreamRecord(ctx sdk.Context, streamRecord *types.StreamRe
214215
return fmt.Errorf("check and force settle failed, err: %w", err)
215216
}
216217
}
217-
settleTimestamp = currentTimestamp - int64(params.ForcedSettleTime) + payDuration.Int64()
218+
settleTimestampFull := sdkmath.NewInt(currentTimestamp).
219+
Sub(sdkmath.NewIntFromUint64(params.ForcedSettleTime)).
220+
Add(payDuration)
221+
if settleTimestampFull.GT(sdkmath.NewInt(math.MaxInt64)) {
222+
// The settle timestamp is stored as int64; a pay duration beyond
223+
// MaxInt64 seconds (~2.9e11 years) cannot be represented.
224+
//
225+
// Reject only a user-initiated deposit — a positive static-balance
226+
// change with no rate change. That depositor is over-funding past what
227+
// we can represent and can simply deposit less. Every other path must
228+
// NOT error here:
229+
// - forced EndBlocker updates: returning an error would panic the
230+
// chain in abci.go ("should not happen");
231+
// - rate decreases from object deletion/discontinue, and lazy
232+
// re-pricing when the SP price falls: rejecting a legitimate
233+
// delete/re-price would be wrong.
234+
// Those saturate the settle timestamp to MaxInt64. It is only a
235+
// scheduling hint for the auto-settle queue and is recomputed on the
236+
// next balance change or bucket touch, so capping it is loss-free.
237+
isUserDeposit := !forced && change.StaticBalanceChange.IsPositive() && change.RateChange.IsZero()
238+
if isUserDeposit {
239+
return types.ErrSettleTimestampOverflow.Wrapf(
240+
"account %s pay duration %s exceeds int64 range; reduce deposit",
241+
streamRecord.Account, payDuration.String())
242+
}
243+
ctx.Logger().Error("settle timestamp overflow, capping at MaxInt64",
244+
"account", streamRecord.Account,
245+
"payDuration", payDuration.String(),
246+
"forced", forced,
247+
"height", ctx.BlockHeight())
248+
settleTimestamp = math.MaxInt64
249+
} else {
250+
settleTimestamp = settleTimestampFull.Int64()
251+
}
218252
}
219253
k.UpdateAutoSettleRecord(ctx, sdk.MustAccAddressFromHex(streamRecord.Account), streamRecord.SettleTimestamp, settleTimestamp)
220254
streamRecord.SettleTimestamp = settleTimestamp
@@ -401,7 +435,18 @@ func (k Keeper) TryResumeStreamRecord(ctx sdk.Context, streamRecord *types.Strea
401435
}
402436

403437
prevSettleTime := streamRecord.SettleTimestamp
404-
streamRecord.SettleTimestamp = now + streamRecord.StaticBalance.Quo(totalRate.Abs()).Int64() - int64(forcedSettleTime)
438+
settleTimestampFull := sdkmath.NewInt(now).
439+
Add(streamRecord.StaticBalance.Quo(totalRate.Abs())).
440+
Sub(sdkmath.NewIntFromUint64(forcedSettleTime))
441+
if settleTimestampFull.GT(sdkmath.NewInt(math.MaxInt64)) {
442+
// TryResumeStreamRecord is only reachable from MsgDeposit, never from an
443+
// EndBlocker path. Reject the deposit so the caller can choose a smaller
444+
// amount rather than silently creating an unrepresentable settle timestamp.
445+
return types.ErrSettleTimestampOverflow.Wrapf(
446+
"account %s: deposit would fund account beyond the representable future; reduce deposit",
447+
streamRecord.Account)
448+
}
449+
streamRecord.SettleTimestamp = settleTimestampFull.Int64()
405450
streamRecord.BufferBalance = expectedBalanceToResume
406451
streamRecord.StaticBalance = streamRecord.StaticBalance.Sub(expectedBalanceToResume)
407452
streamRecord.CrudTimestamp = now

x/payment/keeper/stream_record_test.go

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package keeper_test
22

33
import (
44
"errors"
5+
"math"
56
"testing"
67
"time"
78

@@ -826,6 +827,185 @@ func TestSettleStreamRecord(t *testing.T) {
826827
require.Equal(t, userStreamRecord2.CrudTimestamp, streamRecord.CrudTimestamp+seconds)
827828
}
828829

830+
// TestUpdateStreamRecord_SettleTimestampOverflow_ForcedSaturates reproduces the
831+
// original chain-halt: a forced EndBlocker update (ForceDeleteObject during
832+
// discontinue) decreases the netflow rate on a large-balance account, pushing
833+
// payDuration past MaxInt64. The EndBlocker cannot surface an error (abci.go
834+
// panics on any error from DeleteDiscontinueObjectsUntil), so the settle
835+
// timestamp must saturate to MaxInt64 instead of overflowing.
836+
func TestUpdateStreamRecord_SettleTimestampOverflow_ForcedSaturates(t *testing.T) {
837+
k, ctx, _ := makePaymentKeeper(t)
838+
ctx = ctx.WithBlockTime(time.Unix(1000, 0))
839+
ctx = ctx.WithValue(types.ForceUpdateStreamRecordKey, true)
840+
841+
params := k.GetParams(ctx)
842+
// Buffer reserved for the starting rate (-2); after the rate drops to -1 the
843+
// buffer recomputes to 1×ReserveTime. payDuration overflows either way.
844+
bufferBalance := sdkmath.NewIntFromUint64(params.VersionedParams.ReserveTime).MulRaw(2)
845+
846+
user := sample.RandAccAddress()
847+
sr := &types.StreamRecord{
848+
Account: user.String(),
849+
Status: types.STREAM_ACCOUNT_STATUS_ACTIVE,
850+
StaticBalance: sdkmath.NewIntFromUint64(math.MaxUint64),
851+
BufferBalance: bufferBalance,
852+
LockBalance: sdkmath.ZeroInt(),
853+
NetflowRate: sdkmath.NewInt(-2),
854+
FrozenNetflowRate: sdkmath.ZeroInt(),
855+
CrudTimestamp: ctx.BlockTime().Unix(),
856+
}
857+
858+
// Forced rate decrease (-2 → -1), as when an object is force-deleted.
859+
change := types.NewDefaultStreamRecordChangeWithAddr(user).WithRateChange(sdkmath.NewInt(1))
860+
require.NotPanics(t, func() {
861+
err := k.UpdateStreamRecord(ctx, sr, change)
862+
require.NoError(t, err, "forced path must not return an error (would panic EndBlocker)")
863+
})
864+
require.Equal(t, int64(math.MaxInt64), sr.SettleTimestamp,
865+
"forced overflow must saturate to MaxInt64, not panic")
866+
}
867+
868+
// TestUpdateStreamRecord_SettleTimestampOverflow_UserDepositRejected covers the
869+
// user-initiated deposit path: a positive static-balance change with no rate
870+
// change that pushes payDuration past MaxInt64 is rejected with
871+
// ErrSettleTimestampOverflow, so the depositor can simply deposit less. This is
872+
// the one path where the degenerate state is genuinely user-created.
873+
func TestUpdateStreamRecord_SettleTimestampOverflow_UserDepositRejected(t *testing.T) {
874+
k, ctx, _ := makePaymentKeeper(t)
875+
ctx = ctx.WithBlockTime(time.Unix(1000, 0))
876+
// No ForceUpdateStreamRecordKey → forced=false (user-initiated path).
877+
878+
params := k.GetParams(ctx)
879+
bufferBalance := sdkmath.NewIntFromUint64(params.VersionedParams.ReserveTime)
880+
881+
user := sample.RandAccAddress()
882+
sr := &types.StreamRecord{
883+
Account: user.String(),
884+
Status: types.STREAM_ACCOUNT_STATUS_ACTIVE,
885+
StaticBalance: sdkmath.NewIntFromUint64(math.MaxUint64),
886+
BufferBalance: bufferBalance,
887+
LockBalance: sdkmath.ZeroInt(),
888+
NetflowRate: sdkmath.NewInt(-1),
889+
FrozenNetflowRate: sdkmath.ZeroInt(),
890+
CrudTimestamp: ctx.BlockTime().Unix(),
891+
}
892+
893+
// A real deposit: positive static-balance change, no rate change.
894+
change := types.NewDefaultStreamRecordChangeWithAddr(user).
895+
WithStaticBalanceChange(sdkmath.NewInt(1_000))
896+
err := k.UpdateStreamRecord(ctx, sr, change)
897+
require.ErrorIs(t, err, types.ErrSettleTimestampOverflow,
898+
"a deposit that overflows the settle timestamp must be rejected")
899+
}
900+
901+
// TestUpdateStreamRecord_SettleTimestampOverflow_RateDecreaseSaturates covers a
902+
// non-forced rate decrease — a user MsgDeleteObject, or lazy re-pricing when the
903+
// SP price falls. Removing rate increases payDuration and can overflow int64, but
904+
// this is a legitimate action (not over-funding), so it must NOT be rejected: the
905+
// settle timestamp saturates to MaxInt64 and the operation succeeds. This is the
906+
// asymmetry B fixes — a user can always delete their own object.
907+
func TestUpdateStreamRecord_SettleTimestampOverflow_RateDecreaseSaturates(t *testing.T) {
908+
k, ctx, _ := makePaymentKeeper(t)
909+
ctx = ctx.WithBlockTime(time.Unix(1000, 0))
910+
// No ForceUpdateStreamRecordKey → forced=false (user-initiated path).
911+
912+
params := k.GetParams(ctx)
913+
bufferBalance := sdkmath.NewIntFromUint64(params.VersionedParams.ReserveTime).MulRaw(2)
914+
915+
user := sample.RandAccAddress()
916+
sr := &types.StreamRecord{
917+
Account: user.String(),
918+
Status: types.STREAM_ACCOUNT_STATUS_ACTIVE,
919+
StaticBalance: sdkmath.NewIntFromUint64(math.MaxUint64),
920+
BufferBalance: bufferBalance,
921+
LockBalance: sdkmath.ZeroInt(),
922+
NetflowRate: sdkmath.NewInt(-2),
923+
FrozenNetflowRate: sdkmath.ZeroInt(),
924+
CrudTimestamp: ctx.BlockTime().Unix(),
925+
}
926+
927+
// Rate decrease (-2 → -1), as when a user deletes an object. No static-balance
928+
// change, so this is not a user deposit and must not be rejected.
929+
change := types.NewDefaultStreamRecordChangeWithAddr(user).WithRateChange(sdkmath.NewInt(1))
930+
require.NotPanics(t, func() {
931+
err := k.UpdateStreamRecord(ctx, sr, change)
932+
require.NoError(t, err, "a legitimate rate decrease must not be rejected")
933+
})
934+
require.Equal(t, int64(math.MaxInt64), sr.SettleTimestamp,
935+
"rate-decrease overflow must saturate to MaxInt64, not reject")
936+
}
937+
938+
// TestTryResumeStreamRecord_SettleTimestampInt64Overflow covers the Deposit
939+
// path to a frozen account. TryResumeStreamRecord is only reachable from
940+
// MsgDeposit (never from an EndBlocker), so an overflow must be rejected with
941+
// ErrSettleTimestampOverflow; the user should reduce their deposit amount.
942+
func TestTryResumeStreamRecord_SettleTimestampInt64Overflow(t *testing.T) {
943+
k, ctx, _ := makePaymentKeeper(t)
944+
ctx = ctx.WithBlockTime(time.Unix(1000, 0))
945+
946+
user := sample.RandAccAddress()
947+
sr := &types.StreamRecord{
948+
Account: user.String(),
949+
Status: types.STREAM_ACCOUNT_STATUS_FROZEN,
950+
StaticBalance: sdkmath.ZeroInt(),
951+
BufferBalance: sdkmath.ZeroInt(),
952+
LockBalance: sdkmath.ZeroInt(),
953+
NetflowRate: sdkmath.ZeroInt(),
954+
FrozenNetflowRate: sdkmath.NewInt(-1),
955+
OutFlowCount: 1,
956+
}
957+
k.SetStreamRecord(ctx, sr)
958+
959+
deposit := sdkmath.NewIntFromUint64(math.MaxUint64)
960+
err := k.TryResumeStreamRecord(ctx, sr, deposit)
961+
require.ErrorIs(t, err, types.ErrSettleTimestampOverflow,
962+
"deposit that overflows settle timestamp must be rejected, not silently absorbed")
963+
}
964+
965+
// TestUpdateStreamRecord_SettleTimestampSilentWrap catches the secondary overflow:
966+
// even when payDuration < MaxInt64 (so Int64() would not have panicked), the full
967+
// expression currentTimestamp - forcedSettleTime + payDuration can itself overflow
968+
// int64 and silently wrap to a large negative value. A negative settle timestamp
969+
// makes the account look overdue and drives repeated force-settle attempts.
970+
func TestUpdateStreamRecord_SettleTimestampSilentWrap(t *testing.T) {
971+
k, ctx, _ := makePaymentKeeper(t)
972+
params := k.GetParams(ctx)
973+
reserveTime := sdkmath.NewIntFromUint64(params.VersionedParams.ReserveTime)
974+
975+
timestamp := int64(1_000_000_000)
976+
ctx = ctx.WithBlockTime(time.Unix(timestamp, 0))
977+
ctx = ctx.WithValue(types.ForceUpdateStreamRecordKey, true)
978+
979+
// payDuration = MaxInt64 - 500 fits in int64, so Int64() would not panic
980+
// pre-fix. But timestamp + payDuration - forcedSettleTime overflows int64.
981+
payDuration := sdkmath.NewInt(math.MaxInt64).SubRaw(500)
982+
// With |rate|=1 and bufferBal=reserveTime: staticBal = payDuration - reserveTime.
983+
staticBal := payDuration.Sub(reserveTime)
984+
bufferBal := reserveTime
985+
986+
user := sample.RandAccAddress()
987+
sr := &types.StreamRecord{
988+
Account: user.String(),
989+
Status: types.STREAM_ACCOUNT_STATUS_ACTIVE,
990+
StaticBalance: staticBal,
991+
BufferBalance: bufferBal,
992+
LockBalance: sdkmath.ZeroInt(),
993+
NetflowRate: sdkmath.NewInt(-1),
994+
FrozenNetflowRate: sdkmath.ZeroInt(),
995+
CrudTimestamp: timestamp,
996+
}
997+
998+
change := types.NewDefaultStreamRecordChangeWithAddr(user)
999+
require.NotPanics(t, func() {
1000+
err := k.UpdateStreamRecord(ctx, sr, change)
1001+
require.NoError(t, err)
1002+
})
1003+
require.Equal(t, int64(math.MaxInt64), sr.SettleTimestamp,
1004+
"settle timestamp must saturate to MaxInt64 when full expression overflows int64")
1005+
require.Greater(t, sr.SettleTimestamp, int64(0),
1006+
"settle timestamp must not silently wrap to negative")
1007+
}
1008+
8291009
func TestAutoForceSettle(t *testing.T) {
8301010
keeper, ctx, depKeepers := makePaymentKeeper(t)
8311011
t.Logf("depKeepers: %+v", depKeepers)

x/payment/types/errors.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,5 @@ var (
2020
ErrIncorrectWithdrawAmount = errorsmod.Register(ModuleName, 1211, "the withdrawal amount is not equal to the delayed one")
2121
ErrNotReachTimeLockDuration = errorsmod.Register(ModuleName, 1212, "the withdrawal does not reach to the delayed duration")
2222
ErrExistsDelayedWithdrawal = errorsmod.Register(ModuleName, 1213, "delayed withdrawal already exists")
23+
ErrSettleTimestampOverflow = errorsmod.Register(ModuleName, 1214, "settle timestamp overflow: deposit would fund the account beyond the representable future")
2324
)

0 commit comments

Comments
 (0)