Skip to content
Open
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
6 changes: 6 additions & 0 deletions docs/release-notes/release-notes-0.8.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@

## RPC Updates

- [PR#1766](https://github.com/lightninglabs/taproot-assets/pull/1766):
Introduce structured price oracle errors that allow oracles to return
specific error codes marked as public or private, as is appropriate.
Also, add a new error code that oracles can use to indicate that an
asset is unsupported.

- [PR#1841](https://github.com/lightninglabs/taproot-assets/pull/1841): Remove
the defaultMacaroonWhitelist map and inline its entries directly
into the conditional logic within MacaroonWhitelist. This ensures that
Expand Down
51 changes: 39 additions & 12 deletions rfq/negotiator.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rfq

import (
"errors"
"fmt"
"sync"
"time"
Expand Down Expand Up @@ -146,17 +147,17 @@ func (n *Negotiator) queryBuyFromPriceOracle(assetSpecifier asset.Specifier,
}

// Now we will check for an error in the response from the price oracle.
// If present, we will convert it to a string and return it as an error.
// If present, we will relay it with context.
if oracleResponse.Err != nil {
return nil, fmt.Errorf("failed to query price oracle for "+
"buy price: %s", oracleResponse.Err)
"buy price: %w", oracleResponse.Err)
}

// By this point, the price oracle did not return an error or a buy
// price. We will therefore return an error.
if oracleResponse.AssetRate.Rate.ToUint64() == 0 {
return nil, fmt.Errorf("price oracle did not specify a " +
"buy price")
return nil, fmt.Errorf("failed to query price oracle for " +
"buy price: price oracle didn't specify a price")
}

// TODO(ffranr): Check that the buy price is reasonable.
Expand Down Expand Up @@ -282,17 +283,17 @@ func (n *Negotiator) querySellFromPriceOracle(assetSpecifier asset.Specifier,
}

// Now we will check for an error in the response from the price oracle.
// If present, we will convert it to a string and return it as an error.
// If present, we will relay it with context.
if oracleResponse.Err != nil {
return nil, fmt.Errorf("failed to query price oracle for "+
"sell price: %s", oracleResponse.Err)
"sell price: %w", oracleResponse.Err)
}

// By this point, the price oracle did not return an error or a sell
// price. We will therefore return an error.
if oracleResponse.AssetRate.Rate.Coefficient.ToUint64() == 0 {
return nil, fmt.Errorf("price oracle did not specify an " +
"asset to BTC rate")
return nil, fmt.Errorf("failed to query price oracle for " +
"sell price: price oracle didn't specify a price")
}

// TODO(ffranr): Check that the sell price is reasonable.
Expand Down Expand Up @@ -372,10 +373,12 @@ func (n *Negotiator) HandleIncomingBuyRequest(
peerID, request.PriceOracleMetadata, IntentRecvPayment,
)
if err != nil {
// Send a reject message to the peer.
// Construct an appropriate RejectErr based on
// the oracle's response, and send it to the
// peer.
msg := rfqmsg.NewReject(
request.Peer, request.ID,
rfqmsg.ErrUnknownReject,
customRejectErr(err),
)
sendOutgoingMsg(msg)

Expand Down Expand Up @@ -473,10 +476,12 @@ func (n *Negotiator) HandleIncomingSellRequest(
peerID, request.PriceOracleMetadata, IntentPayInvoice,
)
if err != nil {
// Send a reject message to the peer.
// Construct an appropriate RejectErr based on
// the oracle's response, and send it to the
// peer.
msg := rfqmsg.NewReject(
request.Peer, request.ID,
rfqmsg.ErrUnknownReject,
customRejectErr(err),
)
sendOutgoingMsg(msg)

Expand All @@ -495,6 +500,28 @@ func (n *Negotiator) HandleIncomingSellRequest(
return nil
}

// customRejectErr creates a RejectErr with an opaque rejection code and a
// custom message based on an error response from a price oracle.
func customRejectErr(err error) rfqmsg.RejectErr {
var oracleError *OracleError

// Check if the error chain contains an OracleError, and return an
// opaque rejection error if not.
if !errors.As(err, &oracleError) {
return rfqmsg.ErrUnknownReject
}

// If the price oracle has indicated that this error should not be
// forwarded to peers, then return an opaque rejection error.
if !oracleError.Public {
return rfqmsg.ErrUnknownReject
}

// Otherwise, return a rejection error that relays the price oracle's
// message.
return rfqmsg.NewRejectErr(oracleError.Msg)
}

// HandleOutgoingSellOrder handles an outgoing sell order by constructing sell
// requests and passing them to the outgoing messages channel. These requests
// are sent to peers.
Expand Down
43 changes: 40 additions & 3 deletions rfq/oracle.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,30 @@ const (
// service.
type OracleError struct {
// Code is a code which uniquely identifies the error type.
Code uint8
Code OracleErrorCode

// Public is a flag that indicates the error can be relayed to
// peers.
Public bool

// Msg is a human-readable error message.
Msg string
}

// OracleErrorCode uniquely identifies the kinds of error an oracle may
// return.
type OracleErrorCode uint8

const (
// UnspecifiedOracleErrorCode represents the case where the oracle has
// declined to give a more specific reason for the error.
UnspecifiedOracleErrorCode OracleErrorCode = iota

// UnsupportedAssetOracleErrorCode represents the case in which an
// oracle does not provide quotes for the requested asset.
UnsupportedAssetOracleErrorCode
)
Comment on lines +104 to +112
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment here re iota


// Error returns a human-readable string representation of the error.
func (o *OracleError) Error() string {
// Sanitise price oracle error message by truncating to 255 characters.
Expand Down Expand Up @@ -381,7 +399,9 @@ func (r *RpcPriceOracle) QuerySellPrice(ctx context.Context,

return &OracleResponse{
Err: &OracleError{
Msg: result.Error.Message,
Msg: result.Error.Message,
Public: result.Error.Public,
Code: marshallErrorCode(result.Error.Code),
},
}, nil

Expand All @@ -390,6 +410,21 @@ func (r *RpcPriceOracle) QuerySellPrice(ctx context.Context,
}
}

// marshallErrorCode marshalls an over-the-wire error code into an
// OracleErrorCode.
func marshallErrorCode(code oraclerpc.ErrorCode) OracleErrorCode {
switch code {
case oraclerpc.ErrorCode_UNSPECIFIED_ORACLE_ERROR_CODE:
return UnspecifiedOracleErrorCode

case oraclerpc.ErrorCode_UNSUPPORTED_ASSET_ORACLE_ERROR_CODE:
return UnsupportedAssetOracleErrorCode

default:
return UnspecifiedOracleErrorCode
}
}

// QueryBuyPrice returns a buy price for the given asset amount.
func (r *RpcPriceOracle) QueryBuyPrice(ctx context.Context,
assetSpecifier asset.Specifier, assetMaxAmt fn.Option[uint64],
Expand Down Expand Up @@ -492,7 +527,9 @@ func (r *RpcPriceOracle) QueryBuyPrice(ctx context.Context,

return &OracleResponse{
Err: &OracleError{
Msg: result.Error.Message,
Msg: result.Error.Message,
Public: result.Error.Public,
Code: marshallErrorCode(result.Error.Code),
},
}, nil

Expand Down
28 changes: 24 additions & 4 deletions rfqmsg/reject.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,22 +76,42 @@ func (v *RejectErr) Record() tlv.Record {
)
}

const (
// UnspecifiedRejectCode indicates that a request-for-quote was
// rejected, without necessarily providing any further detail as to
// why.
UnspecifiedRejectCode uint8 = iota

// UnavailableRejectCode indicates that a request-for-quote was
// rejected as a price oracle was unavailable.
UnavailableRejectCode
)
Comment on lines +79 to +88
Copy link
Contributor

Choose a reason for hiding this comment

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

It’s better, in my view, not to use iota. Reordering the const fields becomes a silent breaking change, and it’s easy to miss in review, especially as more error types are added.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we turn this set of const values into a RejectCode enum type? We could then use it in RejectErr instead of a plain uint8. But I can see that just adding the const values is a win for now. So I wont block approve on this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should make the name more specific than UnavailableRejectCode. Something like PriceOracleUnavailableRejectCode might be clearer. It’s long, but many things could be unavailable, so the extra clarity feels worthwhile.


var (
// ErrUnknownReject is the error code for when the quote is rejected
// for an unspecified reason.
ErrUnknownReject = RejectErr{
Code: 0,
Code: UnspecifiedRejectCode,
Msg: "unknown reject error",
}

// ErrPriceOracleUnavailable is the error code for when the price oracle
// is unavailable.
// ErrPriceOracleUnavailable is the error code for when the price
// oracle is unavailable.
ErrPriceOracleUnavailable = RejectErr{
Code: 1,
Code: UnavailableRejectCode,
Msg: "price oracle unavailable",
}
)

// NewRejectErr produces the "unknown" error code, but pairs it with a
// custom error message.
func NewRejectErr(msg string) RejectErr {
return RejectErr{
Code: UnspecifiedRejectCode,
Msg: msg,
}
}

const (
// latestRejectVersion is the latest supported reject wire message data
// field version.
Expand Down
Loading
Loading