Skip to content
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

feat(rfq-relayer): make QuoteWidthBps a token-level param [SLT-354] #3305

Merged
merged 9 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion services/rfq/relayer/quoter/quoter.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,7 @@ func (m *Manager) getDestAmount(parentCtx context.Context, originAmount *big.Int
if err != nil {
return nil, fmt.Errorf("error getting quote offset bps: %w", err)
}
quoteWidthBps, err := m.config.GetQuoteWidthBps(input.DestChainID)
quoteWidthBps, err := m.config.GetQuoteWidthBps(input.DestChainID, tokenName)
if err != nil {
return nil, fmt.Errorf("error getting quote width bps: %w", err)
}
Expand Down
9 changes: 4 additions & 5 deletions services/rfq/relayer/quoter/quoter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,12 +354,12 @@ func (s *QuoterSuite) TestGetDestAmount() {
DestBalance: balance,
}
setQuoteParams := func(originQuoteOffsetBps, destQuoteOffsetBps, quoteWidthBps float64) {
s.config.BaseChainConfig.QuoteWidthBps = quoteWidthBps
tokenCfg := s.config.Chains[origin].Tokens["USDC"]
tokenCfg.QuoteOffsetBps = originQuoteOffsetBps
s.config.Chains[origin].Tokens["USDC"] = tokenCfg
tokenCfg = s.config.Chains[dest].Tokens["USDC"]
tokenCfg.QuoteOffsetBps = destQuoteOffsetBps
tokenCfg.QuoteWidthBps = quoteWidthBps
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider setting QuoteWidthBps for the origin token configuration

In the setQuoteParams function, QuoteWidthBps is being set only for the destination token configuration. If QuoteWidthBps is a token-level parameter that affects both origin and destination tokens, consider setting it for the origin token configuration as well. This ensures comprehensive testing and consistent behavior across all tokens.


🛠️ Refactor suggestion

Use integer types for basis points parameters

The setQuoteParams function accepts basis points parameters (originQuoteOffsetBps, destQuoteOffsetBps, quoteWidthBps) as float64. Since basis points represent integer values (1 basis point = 0.01%), consider using integer types (e.g., int or uint) for these parameters. This prevents potential floating-point precision issues and more accurately represents the discrete nature of basis points.

s.config.Chains[dest].Tokens["USDC"] = tokenCfg
s.manager.SetConfig(s.config)
}
Expand Down Expand Up @@ -398,12 +398,11 @@ func (s *QuoterSuite) TestGetDestAmount() {
expectedAmount = big.NewInt(960_000_000)
s.Equal(expectedAmount, destAmount)

// Set QuoteWidthBps to -100, should default to balance.
// Set QuoteWidthBps to -100, should error.
setQuoteParams(0, 0, -100)
destAmount, err = s.manager.GetDestAmount(s.GetTestContext(), balance, "USDC", input)
s.NoError(err)
expectedAmount = balance
s.Equal(expectedAmount, destAmount)
s.Error(err)
s.Nil(destAmount)

// Set origin offset to 100, should return 101% of balance.
setQuoteParams(100, 0, 0)
Expand Down
6 changes: 3 additions & 3 deletions services/rfq/relayer/relconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ type ChainConfig struct {
MinGasToken string `yaml:"min_gas_token"`
// QuotePct is the percent of balance to quote.
QuotePct *float64 `yaml:"quote_pct"`
// QuoteWidthBps is the number of basis points to deduct from the dest amount.
// Note that this parameter is applied on a chain level and must be positive.
QuoteWidthBps float64 `yaml:"quote_width_bps"`
// QuoteFixedFeeMultiplier is the multiplier for the fixed fee, applied when generating quotes.
QuoteFixedFeeMultiplier *float64 `yaml:"quote_fixed_fee_multiplier"`
// RelayFixedFeeMultiplier is the multiplier for the fixed fee, applied when relaying.
Expand Down Expand Up @@ -142,6 +139,9 @@ type TokenConfig struct {
// Note that this value can be positive or negative; if positive it effectively increases the quoted price
// of the given token, and vice versa.
QuoteOffsetBps float64 `yaml:"quote_offset_bps"`
// QuoteWidthBps is the number of basis points to deduct from the dest amount.
// Note that this parameter must be positive.
QuoteWidthBps float64 `yaml:"quote_width_bps"`
// MaxBalance is the maximum balance that should be accumulated for this token on this chain (human-readable units)
MaxBalance *string `yaml:"max_balance"`
}
Expand Down
18 changes: 0 additions & 18 deletions services/rfq/relayer/relconfig/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ func TestChainGetters(t *testing.T) {
L1FeeDestGasEstimate: 40000,
MinGasToken: "1000",
QuotePct: relconfig.NewFloatPtr(0),
QuoteWidthBps: 10,
QuoteFixedFeeMultiplier: relconfig.NewFloatPtr(1.1),
RebalanceConfigs: relconfig.RebalanceConfigs{
Synapse: &relconfig.SynapseCCTPRebalanceConfig{
Expand Down Expand Up @@ -60,7 +59,6 @@ func TestChainGetters(t *testing.T) {
L1FeeDestGasEstimate: 40001,
MinGasToken: "1001",
QuotePct: relconfig.NewFloatPtr(51),
QuoteWidthBps: 11,
QuoteFixedFeeMultiplier: relconfig.NewFloatPtr(1.2),
RebalanceConfigs: relconfig.RebalanceConfigs{
Synapse: &relconfig.SynapseCCTPRebalanceConfig{
Expand Down Expand Up @@ -91,7 +89,6 @@ func TestChainGetters(t *testing.T) {
L1FeeDestGasEstimate: 40000,
MinGasToken: "1000",
QuotePct: relconfig.NewFloatPtr(50),
QuoteWidthBps: 10,
QuoteFixedFeeMultiplier: relconfig.NewFloatPtr(1.1),
Tokens: map[string]relconfig.TokenConfig{
"USDC": {
Expand Down Expand Up @@ -257,20 +254,6 @@ func TestChainGetters(t *testing.T) {
assert.Equal(t, chainVal, 0.)
})

t.Run("GetQuoteWidthBps", func(t *testing.T) {
defaultVal, err := cfg.GetQuoteWidthBps(badChainID)
assert.NoError(t, err)
assert.Equal(t, defaultVal, relconfig.DefaultChainConfig.QuoteWidthBps)

baseVal, err := cfgWithBase.GetQuoteWidthBps(badChainID)
assert.NoError(t, err)
assert.Equal(t, baseVal, cfgWithBase.BaseChainConfig.QuoteWidthBps)

chainVal, err := cfgWithBase.GetQuoteWidthBps(chainID)
assert.NoError(t, err)
assert.Equal(t, chainVal, cfgWithBase.Chains[chainID].QuoteWidthBps)
})

t.Run("GetQuoteFixedFeeMultiplier", func(t *testing.T) {
defaultVal, err := cfg.GetQuoteFixedFeeMultiplier(badChainID)
assert.NoError(t, err)
Expand Down Expand Up @@ -311,7 +294,6 @@ func TestGetQuoteOffset(t *testing.T) {
L1FeeDestGasEstimate: 40000,
MinGasToken: "1000",
QuotePct: relconfig.NewFloatPtr(50),
QuoteWidthBps: 10,
QuoteFixedFeeMultiplier: relconfig.NewFloatPtr(1.1),
Tokens: map[string]relconfig.TokenConfig{
"USDC": {
Expand Down
22 changes: 12 additions & 10 deletions services/rfq/relayer/relconfig/getters.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
DestGasEstimate: 100000,
MinGasToken: "100000000000000000", // 1 ETH
QuotePct: NewFloatPtr(100),
QuoteWidthBps: 0,
QuoteFixedFeeMultiplier: NewFloatPtr(1),
RelayFixedFeeMultiplier: NewFloatPtr(1),
}
Expand Down Expand Up @@ -440,20 +439,23 @@
}

// GetQuoteWidthBps returns the QuoteWidthBps for the given chainID.
func (c Config) GetQuoteWidthBps(chainID int) (value float64, err error) {
rawValue, err := c.getChainConfigValue(chainID, "QuoteWidthBps")
if err != nil {
return value, err
func (c Config) GetQuoteWidthBps(chainID int, tokenName string) (value float64, err error) {
chainCfg, ok := c.Chains[chainID]
if !ok {
return 0, fmt.Errorf("no chain config for chain %d", chainID)

Check warning on line 445 in services/rfq/relayer/relconfig/getters.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/relconfig/getters.go#L442-L445

Added lines #L442 - L445 were not covered by tests
}

value, ok := rawValue.(float64)
tokenCfg, ok := chainCfg.Tokens[tokenName]

Check warning on line 448 in services/rfq/relayer/relconfig/getters.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/relconfig/getters.go#L448

Added line #L448 was not covered by tests
if !ok {
return value, fmt.Errorf("failed to cast QuoteWidthBps to float")
return 0, fmt.Errorf("no token config for chain %d and token %s", chainID, tokenName)

Check warning on line 450 in services/rfq/relayer/relconfig/getters.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/relconfig/getters.go#L450

Added line #L450 was not covered by tests
}
if value <= 0 {
value = DefaultChainConfig.QuoteWidthBps

width := tokenCfg.QuoteWidthBps
if width < 0 {
return 0, fmt.Errorf("quote width bps must be positive: %f", width)

Check warning on line 455 in services/rfq/relayer/relconfig/getters.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/relconfig/getters.go#L453-L455

Added lines #L453 - L455 were not covered by tests
}
return value, nil

return width, nil

Check warning on line 458 in services/rfq/relayer/relconfig/getters.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/relconfig/getters.go#L458

Added line #L458 was not covered by tests
}

// GetQuoteFixedFeeMultiplier returns the QuoteFixedFeeMultiplier for the given chainID.
Expand Down
Loading