Skip to content

Commit e696148

Browse files
authored
Merge pull request #1365 from lightninglabs/rfq-accept-price-qualify-bug
RFQ accept price qualify bug
2 parents 3d2bd03 + 462e07c commit e696148

File tree

4 files changed

+551
-87
lines changed

4 files changed

+551
-87
lines changed

rfq/mock.go

+125
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package rfq
2+
3+
import (
4+
"context"
5+
"slices"
6+
"time"
7+
8+
"github.com/lightninglabs/taproot-assets/asset"
9+
"github.com/lightninglabs/taproot-assets/fn"
10+
"github.com/lightninglabs/taproot-assets/rfqmath"
11+
"github.com/lightninglabs/taproot-assets/rfqmsg"
12+
"github.com/lightningnetwork/lnd/lnwire"
13+
"github.com/stretchr/testify/mock"
14+
)
15+
16+
// MockPriceOracle is a mock implementation of the PriceOracle interface.
17+
// It returns the suggested rate as the exchange rate.
18+
type MockPriceOracle struct {
19+
// Mock is the underlying mock object used to track method invocations
20+
// in tests.
21+
mock.Mock
22+
23+
// expiryDelay is the lifetime of a quote in seconds.
24+
expiryDelay uint64
25+
26+
// assetToBtcRate is the default asset to BTC exchange rate.
27+
assetToBtcRate rfqmath.BigIntFixedPoint
28+
}
29+
30+
// NewMockPriceOracle creates a new mock price oracle.
31+
func NewMockPriceOracle(expiryDelay,
32+
assetRateCoefficient uint64) *MockPriceOracle {
33+
34+
return &MockPriceOracle{
35+
expiryDelay: expiryDelay,
36+
assetToBtcRate: rfqmath.NewBigIntFixedPoint(
37+
assetRateCoefficient, 0,
38+
),
39+
}
40+
}
41+
42+
// NewMockPriceOracleSatPerAsset creates a new mock price oracle with a
43+
// specified satoshis per asset rate.
44+
func NewMockPriceOracleSatPerAsset(expiryDelay uint64,
45+
satsPerAsset uint64) *MockPriceOracle {
46+
47+
return &MockPriceOracle{
48+
expiryDelay: expiryDelay,
49+
50+
// TODO(ffranr): This is incorrect, we should convert
51+
// satoshis per asset to assets per BTC.
52+
assetToBtcRate: rfqmath.NewBigIntFixedPoint(
53+
satsPerAsset, 0,
54+
),
55+
}
56+
}
57+
58+
// QueryAskPrice returns the ask price for the given asset amount.
59+
func (m *MockPriceOracle) QueryAskPrice(ctx context.Context,
60+
assetSpecifier asset.Specifier,
61+
assetMaxAmt fn.Option[uint64],
62+
paymentMaxAmt fn.Option[lnwire.MilliSatoshi],
63+
assetRateHint fn.Option[rfqmsg.AssetRate]) (*OracleResponse, error) {
64+
65+
// Return early with default value if no expected calls are predefined
66+
// for this method.
67+
if !hasExpectedCall(m.ExpectedCalls, "QueryAskPrice") {
68+
// Calculate the rate expiry timestamp.
69+
lifetime := time.Duration(m.expiryDelay) * time.Second
70+
expiry := time.Now().Add(lifetime).UTC()
71+
72+
return &OracleResponse{
73+
AssetRate: rfqmsg.NewAssetRate(
74+
m.assetToBtcRate, expiry,
75+
),
76+
}, nil
77+
}
78+
79+
// If an expected call exist, call normally.
80+
args := m.Called(
81+
ctx, assetSpecifier, assetMaxAmt, paymentMaxAmt, assetRateHint,
82+
)
83+
resp, _ := args.Get(0).(*OracleResponse)
84+
return resp, args.Error(1)
85+
}
86+
87+
// QueryBidPrice returns a bid price for the given asset amount.
88+
func (m *MockPriceOracle) QueryBidPrice(ctx context.Context,
89+
assetSpecifier asset.Specifier,
90+
assetMaxAmt fn.Option[uint64],
91+
paymentMaxAmt fn.Option[lnwire.MilliSatoshi],
92+
assetRateHint fn.Option[rfqmsg.AssetRate]) (*OracleResponse, error) {
93+
94+
// Return early with default value if no expected calls are predefined
95+
// for this method.
96+
if !hasExpectedCall(m.ExpectedCalls, "QueryBidPrice") {
97+
// Calculate the rate expiry timestamp.
98+
lifetime := time.Duration(m.expiryDelay) * time.Second
99+
expiry := time.Now().Add(lifetime).UTC()
100+
101+
return &OracleResponse{
102+
AssetRate: rfqmsg.NewAssetRate(
103+
m.assetToBtcRate, expiry,
104+
),
105+
}, nil
106+
}
107+
108+
// If an expected call exist, call normally.
109+
args := m.Called(
110+
ctx, assetSpecifier, assetMaxAmt, paymentMaxAmt, assetRateHint,
111+
)
112+
resp, _ := args.Get(0).(*OracleResponse)
113+
return resp, args.Error(1)
114+
}
115+
116+
// Ensure that MockPriceOracle implements the PriceOracle interface.
117+
var _ PriceOracle = (*MockPriceOracle)(nil)
118+
119+
// hasExpectedCall checks if the method call has been registered as an expected
120+
// call with the mock object.
121+
func hasExpectedCall(expectedCalls []*mock.Call, method string) bool {
122+
return slices.ContainsFunc(expectedCalls, func(call *mock.Call) bool {
123+
return call.Method == method
124+
})
125+
}

rfq/negotiator.go

+41-19
Original file line numberDiff line numberDiff line change
@@ -600,14 +600,19 @@ func (n *Negotiator) HandleIncomingBuyAccept(msg rfqmsg.BuyAccept,
600600
go func() {
601601
defer n.Wg.Done()
602602

603-
// The buy accept message contains an ask price. This price
604-
// is the price that the peer is willing to accept in order to
605-
// sell the asset that we are buying.
603+
// The buy accept message includes an ask price, which
604+
// represents the amount the peer is willing to accept for the
605+
// asset we are purchasing.
606606
//
607-
// We will sanity check that price by querying our price oracle
608-
// for an ask price. We will then compare the ask price returned
609-
// by the price oracle with the ask price provided by the peer.
610-
assetRate, err := n.queryAskFromPriceOracle(
607+
// To validate this ask, we will query our price oracle for a
608+
// bid price and compare it with the peer's asking price. If the
609+
// two prices fall within an acceptable tolerance, we will
610+
// approve the quote.
611+
//
612+
// When querying the price oracle, we will provide the peer's
613+
// ask price as a hint. The oracle may factor this into its
614+
// calculations to generate a more relevant bid price.
615+
assetRate, err := n.queryBidFromPriceOracle(
611616
msg.Request.AssetSpecifier,
612617
fn.Some(msg.Request.AssetMaxAmt),
613618
fn.None[lnwire.MilliSatoshi](), fn.Some(msg.AssetRate),
@@ -636,8 +641,14 @@ func (n *Negotiator) HandleIncomingBuyAccept(msg rfqmsg.BuyAccept,
636641
return
637642
}
638643

639-
// Ensure that the peer provided price is reasonable given the
640-
// price provided by the price oracle service.
644+
// The price returned by the oracle may not always align with
645+
// our specific interests, depending on the oracle's pricing
646+
// methodology. In other words, it may provide a general market
647+
// price rather than one optimized for our needs.
648+
//
649+
// To account for this, we allow some tolerance in price
650+
// deviation, ensuring that we can accept a reasonable range of
651+
// prices while maintaining flexibility.
641652
tolerance := rfqmath.NewBigIntFromUint64(
642653
n.cfg.AcceptPriceDeviationPpm,
643654
)
@@ -728,17 +739,22 @@ func (n *Negotiator) HandleIncomingSellAccept(msg rfqmsg.SellAccept,
728739
go func() {
729740
defer n.Wg.Done()
730741

731-
// The sell accept message contains a bid price. This price
732-
// is the price that the peer is willing to pay in order to buy
733-
// the asset that we are selling.
742+
// The sell accept message includes a bid price, which
743+
// represents the amount the peer is willing to pay for the
744+
// asset we are selling.
734745
//
735-
// We will sanity check that price by querying our price oracle
736-
// for a bid price. We will then compare the bid price returned
737-
// by the price oracle with the bid price provided by the peer.
738-
assetRate, err := n.queryBidFromPriceOracle(
746+
// To validate this bid, we will query our price oracle for an
747+
// ask price and compare it with the peer's bid. If the two
748+
// prices fall within an acceptable tolerance, we will accept
749+
// the quote.
750+
//
751+
// When querying the price oracle, we will provide the peer's
752+
// bid as a hint. The oracle may incorporate this bid into its
753+
// calculations to determine a more accurate ask price.
754+
assetRate, err := n.queryAskFromPriceOracle(
739755
msg.Request.AssetSpecifier, fn.None[uint64](),
740756
fn.Some(msg.Request.PaymentMaxAmt),
741-
msg.Request.AssetRateHint,
757+
fn.Some(msg.AssetRate),
742758
)
743759
if err != nil {
744760
// The price oracle returned an error. We will return
@@ -764,8 +780,14 @@ func (n *Negotiator) HandleIncomingSellAccept(msg rfqmsg.SellAccept,
764780
return
765781
}
766782

767-
// Ensure that the peer provided price is reasonable given the
768-
// price provided by the price oracle service.
783+
// The price returned by the oracle may not always align with
784+
// our specific interests, depending on the oracle's pricing
785+
// methodology. In other words, it may provide a general market
786+
// price rather than one optimized for our needs.
787+
//
788+
// To account for this, we allow some tolerance in price
789+
// deviation, ensuring that we can accept a reasonable range of
790+
// prices while maintaining flexibility.
769791
tolerance := rfqmath.NewBigIntFromUint64(
770792
n.cfg.AcceptPriceDeviationPpm,
771793
)

0 commit comments

Comments
 (0)