Skip to content

Commit

Permalink
refactor: return base fee in /quote regardless of simulation success.
Browse files Browse the repository at this point in the history
  • Loading branch information
p0mvn committed Nov 4, 2024
1 parent 28f59d8 commit 979a958
Show file tree
Hide file tree
Showing 13 changed files with 109 additions and 83 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

## Unreleased

- #548 - Return base fee in /quote regardless of simulation success.
- #547 - Add /quote simulation for "out given in" single routes.
- #526 - Refactor gas estimation APIs
- #524 - Claimbot
Expand Down
35 changes: 22 additions & 13 deletions domain/cosmos/tx/msg_simulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tx

import (
"context"
"errors"

cosmosclient "github.com/cosmos/cosmos-sdk/client"
txclient "github.com/cosmos/cosmos-sdk/client/tx"
Expand All @@ -12,6 +13,7 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/osmosis-labs/osmosis/v26/app/params"
txfeestypes "github.com/osmosis-labs/osmosis/v26/x/txfees/types"
"github.com/osmosis-labs/sqs/domain"
"github.com/osmosis-labs/sqs/domain/keyring"

gogogrpc "github.com/cosmos/gogoproto/grpc"
Expand Down Expand Up @@ -48,7 +50,7 @@ type MsgSimulator interface {
account *authtypes.BaseAccount,
chainID string,
msg ...sdk.Msg,
) (uint64, sdk.Coin, error)
) domain.QuotePriceInfo
}

// NewGasCalculator creates a new GasCalculator instance.
Expand Down Expand Up @@ -90,13 +92,13 @@ func (c *txGasCalulator) BuildTx(
return nil, err
}

gasAdjusted, feecoin, err := c.PriceMsgs(ctx, txfeesClient, encodingConfig.TxConfig, account, chainID, msg...)
if err != nil {
return nil, err
priceInfo := c.PriceMsgs(ctx, txfeesClient, encodingConfig.TxConfig, account, chainID, msg...)
if priceInfo.Err != "" {
return nil, errors.New(priceInfo.Err)
}

txBuilder.SetGasLimit(gasAdjusted)
txBuilder.SetFeeAmount(sdk.Coins{feecoin})
txBuilder.SetGasLimit(priceInfo.AdjustedGasUsed)
txBuilder.SetFeeAmount(sdk.Coins{priceInfo.FeeCoin})

sigV2 := BuildSignatures(privKey.PubKey(), nil, account.Sequence)
err = txBuilder.SetSignatures(sigV2)
Expand Down Expand Up @@ -145,23 +147,30 @@ func (c *txGasCalulator) SimulateMsgs(encodingConfig cosmosclient.TxConfig, acco
}

// PriceMsgs implements MsgSimulator.
func (c *txGasCalulator) PriceMsgs(ctx context.Context, txfeesClient txfeestypes.QueryClient, encodingConfig cosmosclient.TxConfig, account *authtypes.BaseAccount, chainID string, msg ...sdk.Msg) (uint64, sdk.Coin, error) {
func (c *txGasCalulator) PriceMsgs(ctx context.Context, txfeesClient txfeestypes.QueryClient, encodingConfig cosmosclient.TxConfig, account *authtypes.BaseAccount, chainID string, msg ...sdk.Msg) domain.QuotePriceInfo {
baseDenom, baseFee, err := CalculateFeePrice(ctx, txfeesClient)
if err != nil {
return domain.QuotePriceInfo{Err: err.Error(), BaseFee: baseFee}
}

_, gasAdjusted, err := c.SimulateMsgs(
encodingConfig,
account,
chainID,
msg,
)
if err != nil {
return 0, sdk.Coin{}, err
return domain.QuotePriceInfo{Err: err.Error(), BaseFee: baseFee}
}

feeCoin, err := CalculateFeeCoin(ctx, txfeesClient, gasAdjusted)
if err != nil {
return 0, sdk.Coin{}, err
}
feeAmount := CalculateFeeAmount(baseFee, gasAdjusted)

return gasAdjusted, feeCoin, nil
return domain.QuotePriceInfo{
AdjustedGasUsed: gasAdjusted,
FeeCoin: sdk.Coin{Denom: baseDenom, Amount: feeAmount},
BaseFee: baseFee,
Err: "",
}
}

// CalculateGas calculates the gas required for a transaction using the provided transaction factory and messages.
Expand Down
38 changes: 25 additions & 13 deletions domain/cosmos/tx/msg_simulator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ func TestBuildTx(t *testing.T) {
name: "Error building transaction",
setupMocks: func(calculator mocks.GetCalculateGasMock, txFeesClient *mocks.TxFeesQueryClient, keyring *mocks.Keyring) tx.CalculateGasFn {
keyring.WithGetKey(testKey)
txFeesClient.WithBaseDenom(testDenom, nil)
txFeesClient.WithGetEipBaseFee(testBaseFee, nil)
return calculator(&txtypes.SimulateResponse{}, testGasUsed, assert.AnError)
},
account: &authtypes.BaseAccount{
Expand Down Expand Up @@ -173,6 +175,7 @@ func TestPriceMsgs(t *testing.T) {
msgs []sdk.Msg
expectedGas uint64
expectedFeeCoin sdk.Coin
expectedBaseFee osmomath.Dec
expectedError bool
}{
{
Expand All @@ -189,19 +192,25 @@ func TestPriceMsgs(t *testing.T) {
msgs: []sdk.Msg{testMsg},
expectedGas: testGasUsed,
expectedFeeCoin: sdk.Coin{Denom: testDenom, Amount: osmomath.NewInt(testAmount)},
expectedBaseFee: osmomath.NewDecWithPrec(1, 1),
expectedError: false,
},
{
name: "Error building transaction",
setupMocks: func(calculator mocks.GetCalculateGasMock, txFeesClient *mocks.TxFeesQueryClient, keyring *mocks.Keyring) tx.CalculateGasFn {
keyring.WithGetKey(testKey)
txFeesClient.WithBaseDenom(testDenom, nil)
txFeesClient.WithGetEipBaseFee(testBaseFee, nil)

return calculator(&txtypes.SimulateResponse{}, testGasUsed, assert.AnError)
},
account: &authtypes.BaseAccount{
Sequence: 8,
AccountNumber: 51,
},
expectedError: true,
expectedFeeCoin: sdk.Coin{},
expectedBaseFee: osmomath.Dec{},
expectedError: true,
},
{
name: "Error calculating fee coin",
Expand All @@ -211,11 +220,13 @@ func TestPriceMsgs(t *testing.T) {

return calculator(&txtypes.SimulateResponse{GasInfo: &sdk.GasInfo{GasUsed: 100000}}, testGasUsed, nil)
},
account: testAccount,
chainID: testChainID,
msgs: []sdk.Msg{testMsg},
expectedGas: testGasUsed,
expectedError: true,
account: testAccount,
chainID: testChainID,
msgs: []sdk.Msg{testMsg},
expectedGas: testGasUsed,
expectedFeeCoin: sdk.Coin{},
expectedBaseFee: osmomath.Dec{},
expectedError: true,
},
}

Expand All @@ -227,7 +238,7 @@ func TestPriceMsgs(t *testing.T) {
calculateGasFnMock := tc.setupMocks(mocks.DefaultGetCalculateGasMock, &txFeesClient, &keyring)
msgSimulator := tx.NewGasCalculator(nil, calculateGasFnMock)

gasUsed, feeCoin, err := msgSimulator.PriceMsgs(
priceInfo := msgSimulator.PriceMsgs(
context.Background(),
&txFeesClient,
encodingConfig.TxConfig,
Expand All @@ -237,13 +248,14 @@ func TestPriceMsgs(t *testing.T) {
)

if tc.expectedError {
assert.Error(t, err)
assert.Equal(t, uint64(0), gasUsed)
assert.Equal(t, sdk.Coin{}, feeCoin)
assert.NotEmpty(t, priceInfo.Err)
assert.Equal(t, priceInfo.AdjustedGasUsed, uint64(0))
assert.Equal(t, priceInfo.FeeCoin, sdk.Coin{})
} else {
assert.NoError(t, err)
assert.Equal(t, tc.expectedGas, gasUsed)
assert.Equal(t, tc.expectedFeeCoin, feeCoin)
assert.Empty(t, priceInfo.Err)
assert.Equal(t, tc.expectedGas, priceInfo.AdjustedGasUsed)
assert.Equal(t, tc.expectedFeeCoin, priceInfo.FeeCoin)
assert.Equal(t, tc.expectedBaseFee, priceInfo.BaseFee)
}
})
}
Expand Down
12 changes: 5 additions & 7 deletions domain/cosmos/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,22 +54,20 @@ func BuildSignerData(chainID string, accountNumber, sequence uint64) authsigning
}
}

// CalculateFeeCoin determines the appropriate fee coin for a transaction based on the current base fee
// CalculateFeePrice determines the appropriate fee price for a transaction based on the current base fee
// and the amount of gas used. It queries the base denomination and EIP base fee using the provided gRPC connection.
func CalculateFeeCoin(ctx context.Context, client txfeestypes.QueryClient, gas uint64) (sdk.Coin, error) {
func CalculateFeePrice(ctx context.Context, client txfeestypes.QueryClient) (string, osmomath.Dec, error) {
queryBaseDenomResponse, err := client.BaseDenom(ctx, &txfeestypes.QueryBaseDenomRequest{})
if err != nil {
return sdk.Coin{}, err
return "", osmomath.Dec{}, err
}

queryEipBaseFeeResponse, err := client.GetEipBaseFee(ctx, &txfeestypes.QueryEipBaseFeeRequest{})
if err != nil {
return sdk.Coin{}, err
return "", osmomath.Dec{}, err
}

feeAmount := CalculateFeeAmount(queryEipBaseFeeResponse.BaseFee, gas)

return sdk.NewCoin(queryBaseDenomResponse.BaseDenom, feeAmount), nil
return queryBaseDenomResponse.BaseDenom, queryEipBaseFeeResponse.BaseFee, nil
}

// CalculateFeeAmount calculates the fee amount based on the base fee and gas used.
Expand Down
28 changes: 15 additions & 13 deletions domain/cosmos/tx/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (

"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/types"
sdk "github.com/cosmos/cosmos-sdk/types"
txtypes "github.com/cosmos/cosmos-sdk/types/tx"
signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing"
Expand Down Expand Up @@ -189,13 +188,14 @@ func TestBuildSignerData(t *testing.T) {

func TestCalculateFeeCoin(t *testing.T) {
tests := []struct {
name string
gas uint64
txFeesClient mocks.TxFeesQueryClient
setupMocks func(*mocks.TxFeesQueryClient)
expectedCoin string
expectedAmount osmomath.Int
expectError bool
name string
gas uint64
txFeesClient mocks.TxFeesQueryClient
setupMocks func(*mocks.TxFeesQueryClient)
expectedCoin string
expectedAmount osmomath.Int
expectedBaseFee osmomath.Dec
expectError bool
}{
{
name: "Normal case",
Expand All @@ -204,9 +204,10 @@ func TestCalculateFeeCoin(t *testing.T) {
client.WithBaseDenom("uosmo", nil)
client.WithGetEipBaseFee("0.5", nil)
},
expectedCoin: "uosmo",
expectedAmount: osmomath.NewInt(50000),
expectError: false,
expectedCoin: "uosmo",
expectedAmount: osmomath.NewInt(50000),
expectedBaseFee: osmomath.NewDecWithPrec(5, 1),
expectError: false,
},
{
name: "Error getting base denom",
Expand All @@ -230,13 +231,14 @@ func TestCalculateFeeCoin(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
tt.setupMocks(&tt.txFeesClient)

result, err := sqstx.CalculateFeeCoin(context.TODO(), &tt.txFeesClient, tt.gas)
denom, baseFee, err := sqstx.CalculateFeePrice(context.TODO(), &tt.txFeesClient)

if tt.expectError {
assert.Error(t, err)
} else {
assert.NoError(t, err)
assert.Equal(t, types.NewCoin(tt.expectedCoin, tt.expectedAmount), result)
assert.Equal(t, tt.expectedCoin, denom)
assert.Equal(t, tt.expectedBaseFee, baseFee)
}
})
}
Expand Down
5 changes: 3 additions & 2 deletions domain/mocks/msg_simulator_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
"github.com/osmosis-labs/osmosis/v26/app/params"
txfeestypes "github.com/osmosis-labs/osmosis/v26/x/txfees/types"
"github.com/osmosis-labs/sqs/domain"
sqstx "github.com/osmosis-labs/sqs/domain/cosmos/tx"
"github.com/osmosis-labs/sqs/domain/keyring"
)
Expand Down Expand Up @@ -38,7 +39,7 @@ type MsgSimulatorMock struct {
account *authtypes.BaseAccount,
chainID string,
msg ...sdk.Msg,
) (uint64, sdk.Coin, error)
) domain.QuotePriceInfo
}

var _ sqstx.MsgSimulator = &MsgSimulatorMock{}
Expand Down Expand Up @@ -74,7 +75,7 @@ func (m *MsgSimulatorMock) PriceMsgs(ctx context.Context, txfeesClient txfeestyp
ProtoMessage()
Reset()
String() string
}) (uint64, sdk.Coin, error) {
}) domain.QuotePriceInfo {
if m.PriceMsgsFn != nil {
return m.PriceMsgsFn(ctx, txfeesClient, encodingConfig, account, chainID, msg...)
}
Expand Down
5 changes: 2 additions & 3 deletions domain/mocks/quote_simulator_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,15 @@ import (
"context"

"cosmossdk.io/math"
"github.com/cosmos/cosmos-sdk/types"
"github.com/osmosis-labs/sqs/domain"
)

type QuoteSimulatorMock struct {
SimulateQuoteFn func(ctx context.Context, quote domain.Quote, slippageToleranceMultiplier math.LegacyDec, simulatorAddress string) (uint64, types.Coin, error)
SimulateQuoteFn func(ctx context.Context, quote domain.Quote, slippageToleranceMultiplier math.LegacyDec, simulatorAddress string) domain.QuotePriceInfo
}

// SimulateQuote implements domain.QuoteSimulator.
func (q *QuoteSimulatorMock) SimulateQuote(ctx context.Context, quote domain.Quote, slippageToleranceMultiplier math.LegacyDec, simulatorAddress string) (uint64, types.Coin, error) {
func (q *QuoteSimulatorMock) SimulateQuote(ctx context.Context, quote domain.Quote, slippageToleranceMultiplier math.LegacyDec, simulatorAddress string) domain.QuotePriceInfo {
if q.SimulateQuoteFn != nil {
return q.SimulateQuoteFn(ctx, quote, slippageToleranceMultiplier, simulatorAddress)
}
Expand Down
8 changes: 5 additions & 3 deletions domain/quote_simulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ type QuoteSimulator interface {
// - Only direct (non-split) quotes are supported.
// Retursn error if:
// - Simulator address does not have enough funds to pay for the quote.
SimulateQuote(ctx context.Context, quote Quote, slippageToleranceMultiplier osmomath.Dec, simulatorAddress string) (uint64, sdk.Coin, error)
SimulateQuote(ctx context.Context, quote Quote, slippageToleranceMultiplier osmomath.Dec, simulatorAddress string) QuotePriceInfo
}

type QuotePriceInfo struct {
AdjustedGasUsed uint64 `json:"adjusted_gas_used"`
FeeCoin sdk.Coin `json:"fee_coin"`
AdjustedGasUsed uint64 `json:"adjusted_gas_used"`
FeeCoin sdk.Coin `json:"fee_coin"`
BaseFee osmomath.Dec `json:"base_fee"`
Err string `json:"error"`
}
14 changes: 4 additions & 10 deletions quotesimulator/quote_simulator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/osmosis-labs/osmosis/osmomath"
"github.com/osmosis-labs/osmosis/v26/app/params"
txfeestypes "github.com/osmosis-labs/osmosis/v26/x/txfees/types"
Expand Down Expand Up @@ -35,10 +34,10 @@ func NewQuoteSimulator(msgSimulator tx.MsgSimulator, encodingConfig params.Encod
}

// SimulateQuote implements domain.QuoteSimulator
func (q *quoteSimulator) SimulateQuote(ctx context.Context, quote domain.Quote, slippageToleranceMultiplier osmomath.Dec, simulatorAddress string) (uint64, sdk.Coin, error) {
func (q *quoteSimulator) SimulateQuote(ctx context.Context, quote domain.Quote, slippageToleranceMultiplier osmomath.Dec, simulatorAddress string) domain.QuotePriceInfo {
route := quote.GetRoute()
if len(route) != 1 {
return 0, sdk.Coin{}, fmt.Errorf("route length must be 1, got %d", len(route))
return domain.QuotePriceInfo{Err: fmt.Sprintf("route length must be 1, got %d", len(route))}
}

poolsInRoute := route[0].GetPools()
Expand Down Expand Up @@ -67,16 +66,11 @@ func (q *quoteSimulator) SimulateQuote(ctx context.Context, quote domain.Quote,
// Get the account for the simulator address
baseAccount, err := q.accountQueryClient.GetAccount(ctx, simulatorAddress)
if err != nil {
return 0, sdk.Coin{}, err
return domain.QuotePriceInfo{Err: err.Error()}
}

// Price the message
gasAdjusted, feeCoin, err := q.msgSimulator.PriceMsgs(ctx, q.txFeesClient, q.encodingConfig.TxConfig, baseAccount, q.chainID, swapMsg)
if err != nil {
return 0, sdk.Coin{}, err
}

return gasAdjusted, feeCoin, nil
return q.msgSimulator.PriceMsgs(ctx, q.txFeesClient, q.encodingConfig.TxConfig, baseAccount, q.chainID, swapMsg)
}

var _ domain.QuoteSimulator = &quoteSimulator{}
Loading

0 comments on commit 979a958

Please sign in to comment.