From b2cc88f996c5d703382004541bbe38de244757a0 Mon Sep 17 00:00:00 2001 From: Lucas Date: Fri, 20 Oct 2023 11:43:52 -0400 Subject: [PATCH 1/3] Remove errors from subaccounts keeper functions that don't return errors --- protocol/x/subaccounts/keeper/subaccount.go | 13 +++----- .../x/subaccounts/keeper/subaccount_helper.go | 8 ++--- protocol/x/subaccounts/types/subaccount.go | 33 +++++++++---------- .../x/subaccounts/types/subaccount_test.go | 8 +---- 4 files changed, 26 insertions(+), 36 deletions(-) diff --git a/protocol/x/subaccounts/keeper/subaccount.go b/protocol/x/subaccounts/keeper/subaccount.go index 1763818ed4..e518ea08cd 100644 --- a/protocol/x/subaccounts/keeper/subaccount.go +++ b/protocol/x/subaccounts/keeper/subaccount.go @@ -283,17 +283,17 @@ func (k Keeper) UpdateSubaccounts( } // Apply the updates to perpetual positions. - success, err = UpdatePerpetualPositions( + success = UpdatePerpetualPositions( settledUpdates, perpIdToFundingIndex, ) - if !success || err != nil { + if !success { return success, successPerUpdate, err } // Apply the updates to asset positions. - success, err = UpdateAssetPositions(settledUpdates) - if !success || err != nil { + success = UpdateAssetPositions(settledUpdates) + if !success { return success, successPerUpdate, err } @@ -433,10 +433,7 @@ func (k Keeper) getSettledSubaccount( // division result always rounds towards negative infinity. totalNetSettlementPpm.Div(totalNetSettlementPpm, lib.BigIntOneMillion()), ) - err = newSubaccount.SetUsdcAssetPosition(newUsdcPosition) - if err != nil { - return types.Subaccount{}, nil, err - } + newSubaccount.SetUsdcAssetPosition(newUsdcPosition) return newSubaccount, fundingPayments, nil } diff --git a/protocol/x/subaccounts/keeper/subaccount_helper.go b/protocol/x/subaccounts/keeper/subaccount_helper.go index 23e9dfd36d..76467cba4e 100644 --- a/protocol/x/subaccounts/keeper/subaccount_helper.go +++ b/protocol/x/subaccounts/keeper/subaccount_helper.go @@ -106,7 +106,6 @@ func UpdatePerpetualPositions( perpIdToFundingIndex map[uint32]dtypes.SerializableInt, ) ( success bool, - err error, ) { // Apply the updates. for i, u := range settledUpdates { @@ -164,7 +163,8 @@ func UpdatePerpetualPositions( settledUpdates[i].SettledSubaccount.PerpetualPositions = perpetualPositions } - return true, nil + + return true } // For each settledUpdate in settledUpdates, updates its SettledSubaccount.AssetPositions @@ -173,7 +173,6 @@ func UpdateAssetPositions( settledUpdates []settledUpdate, ) ( success bool, - err error, ) { // Apply the updates. for i, u := range settledUpdates { @@ -226,5 +225,6 @@ func UpdateAssetPositions( settledUpdates[i].SettledSubaccount.AssetPositions = assetPositions } - return true, nil + + return true } diff --git a/protocol/x/subaccounts/types/subaccount.go b/protocol/x/subaccounts/types/subaccount.go index aea7c02edc..fe043f8348 100644 --- a/protocol/x/subaccounts/types/subaccount.go +++ b/protocol/x/subaccounts/types/subaccount.go @@ -73,26 +73,25 @@ func (m *Subaccount) GetUsdcPosition() *big.Int { } // SetUsdcAssetPosition sets the balance of the USDC asset position to `newUsdcPosition`. -// If the absolute value of `newUsdcPosition` cannot be represented in a uint64, -// an error is returned. -func (m *Subaccount) SetUsdcAssetPosition(newUsdcPosition *big.Int) error { - if m != nil { - usdcAssetPosition := m.getUsdcAssetPosition() - if newUsdcPosition == nil || newUsdcPosition.Sign() == 0 { - if usdcAssetPosition != nil { - m.AssetPositions = m.AssetPositions[1:] - } - } else { - if usdcAssetPosition == nil { - usdcAssetPosition = &AssetPosition{ - AssetId: assettypes.AssetUsdc.Id, - } - m.AssetPositions = append([]*AssetPosition{usdcAssetPosition}, m.AssetPositions...) +func (m *Subaccount) SetUsdcAssetPosition(newUsdcPosition *big.Int) { + if m == nil { + return + } + + usdcAssetPosition := m.getUsdcAssetPosition() + if newUsdcPosition == nil || newUsdcPosition.Sign() == 0 { + if usdcAssetPosition != nil { + m.AssetPositions = m.AssetPositions[1:] + } + } else { + if usdcAssetPosition == nil { + usdcAssetPosition = &AssetPosition{ + AssetId: assettypes.AssetUsdc.Id, } - usdcAssetPosition.Quantums = dtypes.NewIntFromBigInt(newUsdcPosition) + m.AssetPositions = append([]*AssetPosition{usdcAssetPosition}, m.AssetPositions...) } + usdcAssetPosition.Quantums = dtypes.NewIntFromBigInt(newUsdcPosition) } - return nil } func (m *Subaccount) getUsdcAssetPosition() *AssetPosition { diff --git a/protocol/x/subaccounts/types/subaccount_test.go b/protocol/x/subaccounts/types/subaccount_test.go index 6339cd6df1..c21cb62775 100644 --- a/protocol/x/subaccounts/types/subaccount_test.go +++ b/protocol/x/subaccounts/types/subaccount_test.go @@ -205,7 +205,6 @@ func TestSetSubaccountQuoteBalance(t *testing.T) { subaccount *types.Subaccount newQuoteBalance *big.Int expectedAssetPositions []*types.AssetPosition - expectedError error }{ "can set nil subaccount": { subaccount: nil, @@ -311,12 +310,7 @@ func TestSetSubaccountQuoteBalance(t *testing.T) { // Run tests. for name, tc := range tests { t.Run(name, func(t *testing.T) { - err := tc.subaccount.SetUsdcAssetPosition(tc.newQuoteBalance) - if tc.expectedError == nil { - require.NoError(t, err) - } else { - require.ErrorIs(t, tc.expectedError, err) - } + tc.subaccount.SetUsdcAssetPosition(tc.newQuoteBalance) if tc.subaccount != nil { require.Equal( t, From ca6354af23c83b9c2b34a065369b5d2a8ab7c3c2 Mon Sep 17 00:00:00 2001 From: Lucas Date: Fri, 20 Oct 2023 11:53:38 -0400 Subject: [PATCH 2/3] Remove success return variable --- protocol/x/subaccounts/keeper/subaccount.go | 10 ++-------- protocol/x/subaccounts/keeper/subaccount_helper.go | 8 -------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/protocol/x/subaccounts/keeper/subaccount.go b/protocol/x/subaccounts/keeper/subaccount.go index e518ea08cd..4a969f54ce 100644 --- a/protocol/x/subaccounts/keeper/subaccount.go +++ b/protocol/x/subaccounts/keeper/subaccount.go @@ -283,19 +283,13 @@ func (k Keeper) UpdateSubaccounts( } // Apply the updates to perpetual positions. - success = UpdatePerpetualPositions( + UpdatePerpetualPositions( settledUpdates, perpIdToFundingIndex, ) - if !success { - return success, successPerUpdate, err - } // Apply the updates to asset positions. - success = UpdateAssetPositions(settledUpdates) - if !success { - return success, successPerUpdate, err - } + UpdateAssetPositions(settledUpdates) // Apply all updates, including a subaccount update event in the Indexer block message // per update and emit a cometbft event for each settled funding payment. diff --git a/protocol/x/subaccounts/keeper/subaccount_helper.go b/protocol/x/subaccounts/keeper/subaccount_helper.go index 76467cba4e..71721cb9a6 100644 --- a/protocol/x/subaccounts/keeper/subaccount_helper.go +++ b/protocol/x/subaccounts/keeper/subaccount_helper.go @@ -104,8 +104,6 @@ func getUpdatedPerpetualPositions( func UpdatePerpetualPositions( settledUpdates []settledUpdate, perpIdToFundingIndex map[uint32]dtypes.SerializableInt, -) ( - success bool, ) { // Apply the updates. for i, u := range settledUpdates { @@ -163,16 +161,12 @@ func UpdatePerpetualPositions( settledUpdates[i].SettledSubaccount.PerpetualPositions = perpetualPositions } - - return true } // For each settledUpdate in settledUpdates, updates its SettledSubaccount.AssetPositions // to reflect settledUpdate.AssetUpdates. func UpdateAssetPositions( settledUpdates []settledUpdate, -) ( - success bool, ) { // Apply the updates. for i, u := range settledUpdates { @@ -225,6 +219,4 @@ func UpdateAssetPositions( settledUpdates[i].SettledSubaccount.AssetPositions = assetPositions } - - return true } From 90075e43987b2717c90648f8b97ab727557ffdcd Mon Sep 17 00:00:00 2001 From: Lucas Date: Fri, 20 Oct 2023 16:16:24 -0400 Subject: [PATCH 3/3] Add TODO --- protocol/x/subaccounts/keeper/subaccount.go | 1 + 1 file changed, 1 insertion(+) diff --git a/protocol/x/subaccounts/keeper/subaccount.go b/protocol/x/subaccounts/keeper/subaccount.go index 4a969f54ce..f387649059 100644 --- a/protocol/x/subaccounts/keeper/subaccount.go +++ b/protocol/x/subaccounts/keeper/subaccount.go @@ -427,6 +427,7 @@ func (k Keeper) getSettledSubaccount( // division result always rounds towards negative infinity. totalNetSettlementPpm.Div(totalNetSettlementPpm, lib.BigIntOneMillion()), ) + // TODO(CLOB-993): Remove this function and use `UpdateAssetPositions` instead. newSubaccount.SetUsdcAssetPosition(newUsdcPosition) return newSubaccount, fundingPayments, nil }