Skip to content

Commit a4be357

Browse files
fazafaza
committed
fix underflow in storage cost calculation
1 parent 96a7fae commit a4be357

1 file changed

Lines changed: 58 additions & 45 deletions

File tree

‎x/storage/keeper/msg_server.go‎

Lines changed: 58 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@ import (
1515
"github.com/gitopia/gitopia/v6/x/storage/types"
1616
)
1717

18-
// Approximate upgrade time of the v6 upgrade which adds storage module
19-
var UpgradeTime = time.Date(2025, time.September, 5, 14, 0, 0, 0, time.UTC)
20-
2118
type msgServer struct {
2219
Keeper
2320
}
@@ -108,14 +105,15 @@ func (k msgServer) UpdateProvider(goCtx context.Context, msg *types.MsgUpdatePro
108105
}
109106

110107
// calculateStorageCharge calculates the charge for storage usage beyond free limit
111-
func (k msgServer) calculateStorageCharge(ctx sdk.Context, currentUsage uint64, newUsage uint64) (sdk.Coin, error) {
108+
func (k msgServer) calculateStorageCharge(ctx sdk.Context, currentUsage uint64, diff int64) (sdk.Coin, error) {
112109
params := k.GetParams(ctx)
113110
freeStorageBytes := params.FreeStorageMb * 1024 * 1024 // Convert MB to bytes
114111

115112
// If current usage is already above free limit, charge for the entire diff
116113
if currentUsage > freeStorageBytes {
117-
diff := newUsage - currentUsage
114+
// Handle negative diff correctly by checking before subtraction
118115
if diff <= 0 {
116+
// Storage decreased or stayed same, no charge
119117
return sdk.NewCoin(params.StoragePricePerGb.Denom, sdk.ZeroInt()), nil
120118
}
121119
// Calculate charge in GB and multiply by price per GB
@@ -125,12 +123,13 @@ func (k msgServer) calculateStorageCharge(ctx sdk.Context, currentUsage uint64,
125123
}
126124

127125
// If new usage is below free limit, no charge
128-
if newUsage <= freeStorageBytes {
126+
newUsage := int64(currentUsage) + diff
127+
if newUsage <= int64(freeStorageBytes) {
129128
return sdk.NewCoin(params.StoragePricePerGb.Denom, sdk.ZeroInt()), nil
130129
}
131130

132131
// Calculate charge for the portion that exceeds free limit
133-
excessBytes := newUsage - freeStorageBytes
132+
excessBytes := newUsage - int64(freeStorageBytes)
134133
excessGb := float64(excessBytes) / (1024 * 1024 * 1024)
135134
chargeAmount := sdk.NewDec(int64(excessGb)).Mul(sdk.NewDecFromInt(params.StoragePricePerGb.Amount))
136135
return sdk.NewCoin(params.StoragePricePerGb.Denom, chargeAmount.TruncateInt()), nil
@@ -181,8 +180,8 @@ func (k msgServer) UpdateRepositoryPackfile(goCtx context.Context, msg *types.Ms
181180
}
182181

183182
// Calculate storage charge
184-
if !k.GetParams(ctx).StoragePricePerGb.IsZero() && repository.UpdatedAt > UpgradeTime.Unix() {
185-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, userQuota.StorageUsed+uint64(diff))
183+
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
184+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, diff)
186185
if err != nil {
187186
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
188187
}
@@ -217,7 +216,16 @@ func (k msgServer) UpdateRepositoryPackfile(goCtx context.Context, msg *types.Ms
217216
existingPackfile.Size_ = msg.Size_
218217
existingPackfile.UpdatedAt = ctx.BlockTime()
219218

220-
userQuota.StorageUsed += uint64(int64(userQuota.StorageUsed) + diff)
219+
// Update storage usage safely handling negative diffs
220+
if diff >= 0 {
221+
userQuota.StorageUsed += uint64(diff)
222+
} else {
223+
if uint64(-diff) > userQuota.StorageUsed {
224+
userQuota.StorageUsed = 0
225+
} else {
226+
userQuota.StorageUsed -= uint64(-diff)
227+
}
228+
}
221229
k.gitopiaKeeper.SetUserQuota(ctx, userQuota)
222230

223231
k.SetPackfile(ctx, existingPackfile)
@@ -226,12 +234,21 @@ func (k msgServer) UpdateRepositoryPackfile(goCtx context.Context, msg *types.Ms
226234
k.IncreaseCidReferenceCount(ctx, msg.Cid)
227235

228236
storageStats := k.GetStorageStats(ctx)
229-
storageStats.TotalPackfileSize += uint64(diff)
237+
// Update storage stats safely handling negative diffs
238+
if diff >= 0 {
239+
storageStats.TotalPackfileSize += uint64(diff)
240+
} else {
241+
if uint64(-diff) > storageStats.TotalPackfileSize {
242+
storageStats.TotalPackfileSize = 0
243+
} else {
244+
storageStats.TotalPackfileSize -= uint64(-diff)
245+
}
246+
}
230247
k.SetStorageStats(ctx, storageStats)
231248
} else {
232249
// Calculate storage charge for new packfile
233-
if !k.GetParams(ctx).StoragePricePerGb.IsZero() && repository.UpdatedAt > UpgradeTime.Unix() {
234-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, userQuota.StorageUsed+msg.Size_)
250+
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
251+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(msg.Size_))
235252
if err != nil {
236253
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
237254
}
@@ -349,19 +366,8 @@ func (k msgServer) UpdateReleaseAssets(goCtx context.Context, msg *types.MsgUpda
349366
}
350367

351368
// Calculate storage charge for the total size difference
352-
if !k.GetParams(ctx).StoragePricePerGb.IsZero() && repository.UpdatedAt > UpgradeTime.Unix() {
353-
var newStorageUsed uint64
354-
if totalSizeDiff >= 0 {
355-
newStorageUsed = userQuota.StorageUsed + uint64(totalSizeDiff)
356-
} else {
357-
if uint64(-totalSizeDiff) > userQuota.StorageUsed {
358-
newStorageUsed = 0
359-
} else {
360-
newStorageUsed = userQuota.StorageUsed - uint64(-totalSizeDiff)
361-
}
362-
}
363-
364-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, newStorageUsed)
369+
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
370+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(totalSizeDiff))
365371
if err != nil {
366372
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
367373
}
@@ -790,8 +796,8 @@ func (k msgServer) UpdateLFSObject(goCtx context.Context, msg *types.MsgUpdateLF
790796
}
791797

792798
// Calculate storage charge for new LFS object
793-
if !k.GetParams(ctx).StoragePricePerGb.IsZero() && repo.UpdatedAt > UpgradeTime.Unix() {
794-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, userQuota.StorageUsed+msg.Size_)
799+
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
800+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(msg.Size_))
795801
if err != nil {
796802
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
797803
}
@@ -1031,7 +1037,7 @@ func (k msgServer) ApproveRepositoryPackfileUpdate(goCtx context.Context, msg *t
10311037

10321038
// Calculate storage charge
10331039
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
1034-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, userQuota.StorageUsed+uint64(diff))
1040+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(diff))
10351041
if err != nil {
10361042
k.RemoveProposedPackfileUpdate(ctx, proposal.Id)
10371043
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
@@ -1075,7 +1081,16 @@ func (k msgServer) ApproveRepositoryPackfileUpdate(goCtx context.Context, msg *t
10751081
existingPackfile.Size_ = proposal.Size_
10761082
existingPackfile.UpdatedAt = ctx.BlockTime()
10771083

1078-
userQuota.StorageUsed += uint64(int64(userQuota.StorageUsed) + diff)
1084+
// Update storage usage safely handling negative diffs
1085+
if diff >= 0 {
1086+
userQuota.StorageUsed += uint64(diff)
1087+
} else {
1088+
if uint64(-diff) > userQuota.StorageUsed {
1089+
userQuota.StorageUsed = 0
1090+
} else {
1091+
userQuota.StorageUsed -= uint64(-diff)
1092+
}
1093+
}
10791094
k.gitopiaKeeper.SetUserQuota(ctx, userQuota)
10801095

10811096
k.SetPackfile(ctx, existingPackfile)
@@ -1084,12 +1099,21 @@ func (k msgServer) ApproveRepositoryPackfileUpdate(goCtx context.Context, msg *t
10841099
k.IncreaseCidReferenceCount(ctx, proposal.Cid)
10851100

10861101
storageStats := k.GetStorageStats(ctx)
1087-
storageStats.TotalPackfileSize += uint64(diff)
1102+
// Update storage stats safely handling negative diffs
1103+
if diff >= 0 {
1104+
storageStats.TotalPackfileSize += uint64(diff)
1105+
} else {
1106+
if uint64(-diff) > storageStats.TotalPackfileSize {
1107+
storageStats.TotalPackfileSize = 0
1108+
} else {
1109+
storageStats.TotalPackfileSize -= uint64(-diff)
1110+
}
1111+
}
10881112
k.SetStorageStats(ctx, storageStats)
10891113
} else {
10901114
// Calculate storage charge for new packfile
10911115
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
1092-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, userQuota.StorageUsed+proposal.Size_)
1116+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(proposal.Size_))
10931117
if err != nil {
10941118
k.RemoveProposedPackfileUpdate(ctx, proposal.Id)
10951119
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
@@ -1526,18 +1550,7 @@ func (k msgServer) ApproveReleaseAssetsUpdate(goCtx context.Context, msg *types.
15261550

15271551
// Calculate storage charge for the total size difference
15281552
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
1529-
var newStorageUsed uint64
1530-
if totalSizeDiff >= 0 {
1531-
newStorageUsed = userQuota.StorageUsed + uint64(totalSizeDiff)
1532-
} else {
1533-
if uint64(-totalSizeDiff) > userQuota.StorageUsed {
1534-
newStorageUsed = 0
1535-
} else {
1536-
newStorageUsed = userQuota.StorageUsed - uint64(-totalSizeDiff)
1537-
}
1538-
}
1539-
1540-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, newStorageUsed)
1553+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(totalSizeDiff))
15411554
if err != nil {
15421555
k.RemoveProposedReleaseAssetsUpdate(ctx, proposal.Id)
15431556
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
@@ -1844,7 +1857,7 @@ func (k msgServer) ApproveLFSObjectUpdate(goCtx context.Context, msg *types.MsgA
18441857

18451858
// Calculate storage charge for new LFS object
18461859
if !k.GetParams(ctx).StoragePricePerGb.IsZero() {
1847-
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, userQuota.StorageUsed+proposal.Size_)
1860+
charge, err := k.calculateStorageCharge(ctx, userQuota.StorageUsed, int64(proposal.Size_))
18481861
if err != nil {
18491862
return nil, fmt.Errorf("failed to calculate storage charge: %v", err)
18501863
}

0 commit comments

Comments
 (0)