Skip to content

Add initial impl - need to fix current test #3249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+40 −4
Open

Add initial impl - need to fix current test #3249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ccf442
Add initial impl - need to fix current test
northstar456 Nov 17, 2025
888f1f6
add nit
northstar456 Nov 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 36 additions & 4 deletions protocol/x/subaccounts/keeper/subaccount.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,33 @@ func (k Keeper) CanUpdateSubaccounts(
return success, successPerUpdate, err
}

func (k Keeper) checkDepositConstraints(
settledUpdates []types.SettledUpdate,
) (
success bool,
successPerUpdate []types.UpdateResult,
) {
success = true
successPerUpdate = make([]types.UpdateResult, len(settledUpdates))

for i, u := range settledUpdates {
for _, assetUpdate := range u.AssetUpdates {
// Reject deposit if the account the account is new (no perp position, no usdc position) and the deposit/transfer
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix typo in comment.

The comment contains a repeated phrase.

Apply this diff:

-			// Reject deposit if the account the account is new (no perp position, no usdc position) and the deposit/transfer
+			// Reject deposit if the account is new (no perp position, no usdc position) and the deposit/transfer
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Reject deposit if the account the account is new (no perp position, no usdc position) and the deposit/transfer
// Reject deposit if the account is new (no perp position, no usdc position) and the deposit/transfer
🤖 Prompt for AI Agents 
In protocol/x/subaccounts/keeper/subaccount.go around line 549, the comment
currently reads "Reject deposit if the account the account is new..." with a
duplicated phrase; remove the extra "the account" so the comment becomes "Reject
deposit if the account is new (no perp position, no usdc position) and the
deposit/transfer" to fix the typo and restore clarity.

// is less than the minimum initial deposit.
// TODO: we need to be careful this doesn't break any existing valid flows.
if assetUpdate.AssetId == assettypes.AssetUsdc.Id &&
assetUpdate.BigQuantumsDelta.Cmp(big.NewInt(types.MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS)) < 0 &&
u.SettledSubaccount.GetUsdcPosition().Sign() == 0 &&
len(u.SettledSubaccount.PerpetualPositions) == 0 {
successPerUpdate[i] = types.ViolatesDepositConstraints
success = false
}
Comment on lines +552 to +558
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add positive delta check for clarity and efficiency.

The comparison assetUpdate.BigQuantumsDelta.Cmp(...) < 0 will evaluate true for any negative delta (e.g., withdrawals). While withdrawals from new accounts would fail elsewhere, checking negative deltas here wastes gas and reduces clarity. The constraint should only apply to incoming deposits/transfers (positive deltas).

Apply this diff:

 			if assetUpdate.AssetId == assettypes.AssetUsdc.Id &&
+				assetUpdate.BigQuantumsDelta.Sign() > 0 &&
 				assetUpdate.BigQuantumsDelta.Cmp(big.NewInt(types.MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS)) < 0 &&
 				u.SettledSubaccount.GetUsdcPosition().Sign() == 0 &&
 				len(u.SettledSubaccount.PerpetualPositions) == 0 {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if assetUpdate.AssetId == assettypes.AssetUsdc.Id &&
assetUpdate.BigQuantumsDelta.Cmp(big.NewInt(types.MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS)) < 0 &&
u.SettledSubaccount.GetUsdcPosition().Sign() == 0 &&
len(u.SettledSubaccount.PerpetualPositions) == 0 {
successPerUpdate[i] = types.ViolatesDepositConstraints
success = false
}
if assetUpdate.AssetId == assettypes.AssetUsdc.Id &&
assetUpdate.BigQuantumsDelta.Sign() > 0 &&
assetUpdate.BigQuantumsDelta.Cmp(big.NewInt(types.MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS)) < 0 &&
u.SettledSubaccount.GetUsdcPosition().Sign() == 0 &&
len(u.SettledSubaccount.PerpetualPositions) == 0 {
successPerUpdate[i] = types.ViolatesDepositConstraints
success = false
}
🤖 Prompt for AI Agents 
In protocol/x/subaccounts/keeper/subaccount.go around lines 552 to 558, the
current check applies the minimum-initial-deposit constraint to any negative
delta as well as positive ones; change it to only run for positive deltas to
improve clarity and gas (first check assetUpdate.BigQuantumsDelta.Sign() > 0,
then compare with types.MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS), keeping the
rest of the condition the same and short-circuiting early to avoid unnecessary
Cmp calls for withdrawals.

}
}

return success, successPerUpdate
}

func (k Keeper) internalCanUpdateSubaccountsWithLeverage(
ctx sdk.Context,
settledUpdates []types.SettledUpdate,
Expand All @@ -557,11 +584,16 @@ func (k Keeper) internalCanUpdateSubaccountsWithLeverage(
return success, successPerUpdate, nil
}

// Block all withdrawals and transfers if either of the following is true within the last
// `WITHDRAWAL_AND_TRANSFERS_BLOCKED_AFTER_NEGATIVE_TNC_SUBACCOUNT_SEEN_BLOCKS`:
// - There was a negative TNC subaccount seen for any of the collateral pools of subaccounts being updated
// - There was a chain outage that lasted at least five minutes.
if updateType == types.Withdrawal || updateType == types.Transfer {
success, successPerUpdate = k.checkDepositConstraints(settledUpdates)
if !success {
return success, successPerUpdate, nil
}

Comment on lines 587 to +592
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Include deposit updates in the minimum deposit gate

Right now we only invoke checkDepositConstraints for Withdrawal and Transfer. Any Deposit update skips this guard entirely, so a user can still open a fresh subaccount with a sub-0.1 USDC deposit via the deposit flow, which defeats the whole intent of MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS. Please run the same check for types.Deposit updates as well.

-	if updateType == types.Withdrawal || updateType == types.Transfer {
+	if updateType == types.Withdrawal || updateType == types.Transfer || updateType == types.Deposit {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if updateType == types.Withdrawal || updateType == types.Transfer {
success, successPerUpdate = k.checkDepositConstraints(settledUpdates)
if !success {
return success, successPerUpdate, nil
}
if updateType == types.Withdrawal || updateType == types.Transfer || updateType == types.Deposit {
success, successPerUpdate = k.checkDepositConstraints(settledUpdates)
if !success {
return success, successPerUpdate, nil
}
🤖 Prompt for AI Agents 
In protocol/x/subaccounts/keeper/subaccount.go around lines 583 to 588, the
deposit minimum check is only applied for Withdrawal and Transfer updates so
Deposit updates bypass the MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS gate; include
types.Deposit in the conditional (or otherwise invoke k.checkDepositConstraints
when updateType == types.Deposit) so deposit updates run the same check, return
the same success, successPerUpdate, nil on failure, and preserve existing
control flow.

// Block all withdrawals and transfers if either of the following is true within the last
// `WITHDRAWAL_AND_TRANSFERS_BLOCKED_AFTER_NEGATIVE_TNC_SUBACCOUNT_SEEN_BLOCKS`:
// - There was a negative TNC subaccount seen for any of the collateral pools of subaccounts being updated
// - There was a chain outage that lasted at least five minutes.
lastBlockNegativeTncSubaccountSeen, negativeTncSubaccountExists, err := k.getLastBlockNegativeSubaccountSeen(
ctx,
settledUpdates,
Expand Down
4 changes: 4 additions & 0 deletions protocol/x/subaccounts/types/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ var updateResultStringMap = map[UpdateResult]string{
WithdrawalsAndTransfersBlocked: "WithdrawalsAndTransfersBlocked",
UpdateCausedError: "UpdateCausedError",
ViolatesIsolatedSubaccountConstraints: "ViolatesIsolatedSubaccountConstraints",
ViolatesDepositConstraints: "ViolatesDepositConstraints",
}

const (
Expand All @@ -71,6 +72,7 @@ const (
WithdrawalsAndTransfersBlocked
UpdateCausedError
ViolatesIsolatedSubaccountConstraints
ViolatesDepositConstraints
)

// Update is used by the subaccounts keeper to allow other modules
Expand Down Expand Up @@ -157,3 +159,5 @@ func (u UpdateType) String() string {
const WITHDRAWAL_AND_TRANSFERS_BLOCKED_AFTER_NEGATIVE_TNC_SUBACCOUNT_SEEN_BLOCKS = 50

const WITHDRAWAL_AND_TRANSFERS_BLOCKED_AFTER_CHAIN_OUTAGE_DURATION = 5 * time.Minute

const MIN_SUBACCOUNT_INITIAL_DEPOSIT_QUANTUMS = 100_000 // 0.1 USDC
Loading