-
Notifications
You must be signed in to change notification settings - Fork 33
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.
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
RFQ: support multiple rebalance methods #2556
Changes from all commits
226b536
478d9ce
8e331bd
5c8a13a
ce61396
893776b
e844c97
d7dd214
928f85f
9059216
a089e1a
f08f8a9
8132d95
c137a48
37c128c
d7abcdb
72df0b1
1108475
d34ae45
93d1a3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,6 +2,7 @@ | |||||
|
||||||
import ( | ||||||
"context" | ||||||
"errors" | ||||||
"fmt" | ||||||
"math/big" | ||||||
"strconv" | ||||||
|
@@ -82,6 +83,9 @@ | |||||
pendingHist metric.Float64Histogram | ||||||
} | ||||||
|
||||||
// ErrUnsupportedChain is the error for an unsupported chain. | ||||||
var ErrUnsupportedChain = errors.New("could not get gas balance for unsupported chain") | ||||||
|
||||||
// GetCommittableBalance gets the committable balances. | ||||||
func (i *inventoryManagerImpl) GetCommittableBalance(ctx context.Context, chainID int, token common.Address, options ...BalanceFetchArgOption) (*big.Int, error) { | ||||||
committableBalances, err := i.GetCommittableBalances(ctx, options...) | ||||||
|
@@ -94,7 +98,7 @@ | |||||
if balance == nil && token == chain.EthAddress { | ||||||
gasBalance, ok := i.gasBalances[chainID] | ||||||
if !ok || gasBalance == nil { | ||||||
return nil, fmt.Errorf("could not get gas balance for chain %d", chainID) | ||||||
return nil, ErrUnsupportedChain | ||||||
} | ||||||
balance = i.gasBalances[chainID] | ||||||
} | ||||||
|
@@ -425,19 +429,19 @@ | |||||
// will be rebalanced. | ||||||
// | ||||||
//nolint:cyclop | ||||||
func (i *inventoryManagerImpl) Rebalance(parentCtx context.Context, chainID int, token common.Address) error { | ||||||
// evaluate the rebalance method | ||||||
method, err := i.cfg.GetRebalanceMethod(chainID, token.Hex()) | ||||||
func (i *inventoryManagerImpl) Rebalance(parentCtx context.Context, chainID int, token common.Address) (err error) { | ||||||
// short circuit if origin does not specify a rebalance method | ||||||
methodOrigin, err := i.cfg.GetRebalanceMethod(chainID, token.Hex()) | ||||||
if err != nil { | ||||||
return fmt.Errorf("could not get rebalance method: %w", err) | ||||||
return fmt.Errorf("could not get origin rebalance method: %w", err) | ||||||
} | ||||||
if method == relconfig.RebalanceMethodNone { | ||||||
if methodOrigin == relconfig.RebalanceMethodNone { | ||||||
return nil | ||||||
} | ||||||
|
||||||
ctx, span := i.handler.Tracer().Start(parentCtx, "Rebalance", trace.WithAttributes( | ||||||
attribute.Int(metrics.ChainID, chainID), | ||||||
attribute.String("token", token.Hex()), | ||||||
attribute.String("rebalance_method", method.String()), | ||||||
)) | ||||||
defer func(err error) { | ||||||
metrics.EndSpanWithErr(span, err) | ||||||
|
@@ -475,9 +479,9 @@ | |||||
} | ||||||
|
||||||
// execute the rebalance | ||||||
manager, ok := i.rebalanceManagers[method] | ||||||
manager, ok := i.rebalanceManagers[rebalance.Method] | ||||||
if !ok { | ||||||
return fmt.Errorf("no rebalance manager for method: %s", method) | ||||||
return fmt.Errorf("no rebalance manager for method: %s", methodOrigin) | ||||||
} | ||||||
err = manager.Execute(ctx, rebalance) | ||||||
if err != nil { | ||||||
|
@@ -506,7 +510,7 @@ | |||||
return nil | ||||||
} | ||||||
|
||||||
//nolint:cyclop,gocognit | ||||||
//nolint:cyclop,gocognit,nilnil | ||||||
func getRebalance(span trace.Span, cfg relconfig.Config, tokens map[int]map[common.Address]*TokenMetadata, chainID int, token common.Address) (rebalance *RebalanceData, err error) { | ||||||
maintenancePct, err := cfg.GetMaintenanceBalancePct(chainID, token.Hex()) | ||||||
if err != nil { | ||||||
|
@@ -522,54 +526,89 @@ | |||||
} | ||||||
} | ||||||
|
||||||
// get total balance for given token across all chains | ||||||
totalBalance := big.NewInt(0) | ||||||
// evaluate the origin and dest of the rebalance based on min/max token balances | ||||||
var destTokenData, originTokenData *TokenMetadata | ||||||
for _, tokenMap := range tokens { | ||||||
for _, tokenData := range tokenMap { | ||||||
if tokenData.Name == rebalanceTokenData.Name { | ||||||
totalBalance.Add(totalBalance, tokenData.Balance) | ||||||
if destTokenData == nil || tokenData.Balance.Cmp(destTokenData.Balance) < 0 { | ||||||
destTokenData = tokenData | ||||||
} | ||||||
if originTokenData == nil || tokenData.Balance.Cmp(originTokenData.Balance) > 0 { | ||||||
originTokenData = tokenData | ||||||
} | ||||||
Comment on lines
+529
to
+539
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optimize the logic for determining the rebalance origin and destination. Consider using a more efficient method to determine the minimum and maximum balances, such as maintaining sorted lists of balances or using priority queues. This could significantly reduce the computational complexity, especially if the number of tokens is large. |
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
// check if any balances are below maintenance threshold | ||||||
var minTokenData, maxTokenData *TokenMetadata | ||||||
for _, tokenMap := range tokens { | ||||||
for _, tokenData := range tokenMap { | ||||||
if tokenData.Name == rebalanceTokenData.Name { | ||||||
if minTokenData == nil || tokenData.Balance.Cmp(minTokenData.Balance) < 0 { | ||||||
minTokenData = tokenData | ||||||
} | ||||||
if maxTokenData == nil || tokenData.Balance.Cmp(maxTokenData.Balance) > 0 { | ||||||
maxTokenData = tokenData | ||||||
} | ||||||
} | ||||||
// if the given chain is not the origin of the rebalance, no need to do anything | ||||||
defer func() { | ||||||
if span != nil { | ||||||
span.SetAttributes( | ||||||
attribute.Int("rebalance_chain_id", chainID), | ||||||
attribute.Int("rebalance_origin", originTokenData.ChainID), | ||||||
attribute.Int("rebalance_dest", destTokenData.ChainID), | ||||||
) | ||||||
} | ||||||
}() | ||||||
if originTokenData.ChainID != chainID { | ||||||
return nil, nil | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a sentinel error instead of returning both - return nil, nil
+ return nil, ErrInvalidRebalance Also applies to: 572-572 Committable suggestion
Suggested change
|
||||||
} | ||||||
|
||||||
// validate the rebalance method pair | ||||||
methodOrigin, err := cfg.GetRebalanceMethod(originTokenData.ChainID, originTokenData.Addr.Hex()) | ||||||
if err != nil { | ||||||
return nil, fmt.Errorf("could not get origin rebalance method: %w", err) | ||||||
} | ||||||
methodDest, err := cfg.GetRebalanceMethod(destTokenData.ChainID, destTokenData.Addr.Hex()) | ||||||
if err != nil { | ||||||
return nil, fmt.Errorf("could not get dest rebalance method: %w", err) | ||||||
} | ||||||
rebalanceMethod := relconfig.CoalesceRebalanceMethods(methodOrigin, methodDest) | ||||||
defer func() { | ||||||
if span != nil { | ||||||
span.SetAttributes(attribute.Int("rebalance_method", int(rebalanceMethod))) | ||||||
span.SetAttributes(attribute.Int("origin_rebalance_method", int(methodOrigin))) | ||||||
span.SetAttributes(attribute.Int("dest_rebalance_method", int(methodDest))) | ||||||
} | ||||||
}() | ||||||
if rebalanceMethod == relconfig.RebalanceMethodNone { | ||||||
return nil, nil | ||||||
} | ||||||
|
||||||
// get the initialPct for the origin chain | ||||||
initialPct, err := cfg.GetInitialBalancePct(maxTokenData.ChainID, maxTokenData.Addr.Hex()) | ||||||
initialPct, err := cfg.GetInitialBalancePct(originTokenData.ChainID, originTokenData.Addr.Hex()) | ||||||
if err != nil { | ||||||
return nil, fmt.Errorf("could not get initial pct: %w", err) | ||||||
} | ||||||
|
||||||
// calculate maintenance threshold relative to total balance | ||||||
totalBalance := big.NewInt(0) | ||||||
for _, tokenMap := range tokens { | ||||||
for _, tokenData := range tokenMap { | ||||||
if tokenData.Name == rebalanceTokenData.Name { | ||||||
totalBalance.Add(totalBalance, tokenData.Balance) | ||||||
} | ||||||
} | ||||||
} | ||||||
maintenanceThresh, _ := new(big.Float).Mul(new(big.Float).SetInt(totalBalance), big.NewFloat(maintenancePct/100)).Int(nil) | ||||||
if span != nil { | ||||||
span.SetAttributes(attribute.Float64("maintenance_pct", maintenancePct)) | ||||||
span.SetAttributes(attribute.Float64("initial_pct", initialPct)) | ||||||
span.SetAttributes(attribute.String("max_token_balance", maxTokenData.Balance.String())) | ||||||
span.SetAttributes(attribute.String("min_token_balance", minTokenData.Balance.String())) | ||||||
span.SetAttributes(attribute.String("max_token_balance", originTokenData.Balance.String())) | ||||||
span.SetAttributes(attribute.String("min_token_balance", destTokenData.Balance.String())) | ||||||
span.SetAttributes(attribute.String("total_balance", totalBalance.String())) | ||||||
span.SetAttributes(attribute.String("maintenance_thresh", maintenanceThresh.String())) | ||||||
} | ||||||
|
||||||
// check if the minimum balance is below the threshold and trigger rebalance | ||||||
if minTokenData.Balance.Cmp(maintenanceThresh) > 0 { | ||||||
if destTokenData.Balance.Cmp(maintenanceThresh) > 0 { | ||||||
return rebalance, nil | ||||||
} | ||||||
|
||||||
// calculate the amount to rebalance vs the initial threshold on origin | ||||||
initialThresh, _ := new(big.Float).Mul(new(big.Float).SetInt(totalBalance), big.NewFloat(initialPct/100)).Int(nil) | ||||||
amount := new(big.Int).Sub(maxTokenData.Balance, initialThresh) | ||||||
amount := new(big.Int).Sub(originTokenData.Balance, initialThresh) | ||||||
|
||||||
// no need to rebalance since amount would not be positive | ||||||
if amount.Cmp(big.NewInt(0)) <= 0 { | ||||||
|
@@ -578,15 +617,15 @@ | |||||
} | ||||||
|
||||||
// filter the rebalance amount by the configured min | ||||||
minAmount := cfg.GetMinRebalanceAmount(maxTokenData.ChainID, maxTokenData.Addr) | ||||||
minAmount := cfg.GetMinRebalanceAmount(originTokenData.ChainID, originTokenData.Addr) | ||||||
if amount.Cmp(minAmount) < 0 { | ||||||
// no need to rebalance | ||||||
//nolint:nilnil | ||||||
return nil, nil | ||||||
} | ||||||
|
||||||
// clip the rebalance amount by the configured max | ||||||
maxAmount := cfg.GetMaxRebalanceAmount(maxTokenData.ChainID, maxTokenData.Addr) | ||||||
maxAmount := cfg.GetMaxRebalanceAmount(originTokenData.ChainID, originTokenData.Addr) | ||||||
if amount.Cmp(maxAmount) > 0 { | ||||||
amount = maxAmount | ||||||
} | ||||||
|
@@ -599,9 +638,10 @@ | |||||
} | ||||||
|
||||||
rebalance = &RebalanceData{ | ||||||
OriginMetadata: maxTokenData, | ||||||
DestMetadata: minTokenData, | ||||||
OriginMetadata: originTokenData, | ||||||
DestMetadata: destTokenData, | ||||||
Amount: amount, | ||||||
Method: rebalanceMethod, | ||||||
} | ||||||
return rebalance, nil | ||||||
} | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor the
getRebalance
function to reduce complexity.Consider breaking down this function into smaller, more manageable functions. This could improve readability and maintainability, especially as the function handles multiple responsibilities such as fetching token metadata, determining rebalance origin and destination, and calculating rebalance amounts.