-
Notifications
You must be signed in to change notification settings - Fork 132
Add example basic price oracle service #1004
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
Conversation
b7994cb
to
6981252
Compare
6981252
to
dc12d95
Compare
if transactionType == oraclerpc.TransactionType_PURCHASE { | ||
// The rate for a purchase transaction is 42,000 milli-satoshi | ||
// per asset unit | ||
rate = 42_000 |
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.
Based on this, I think we need to update the proto comments and other godocs re the "rate tick". Initially it started out at one ten-thousandth, but during the final portion of dev, switching over to units per mSAT made more sense (as that's the smallest amount we can send).
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.
I've updated this comment (and the one below) to state "42,000 asset units per mSAT."
|
||
// Determine the rate based on the transaction type. | ||
var rate uint64 | ||
if transactionType == oraclerpc.TransactionType_PURCHASE { |
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.
Can we add more godoc details to the diff of the two transaction types? Perhaps with a brief walkthrough example? So something like: Alice has X
and wants to pay an invoice, she queries the oracle for a quote with transactionType
Z
.
On second thought, this sounds like something we should add as normal mark down docs, and also in the builder's guide.
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.
In the latest commits I've included an example in the doc for QueryRateTick
.
|
||
// Ensure that the payment asset is BTC. We only support BTC as the | ||
// payment asset in this example. | ||
if !oraclerpc.IsAssetBtc(req.PaymentAsset) { |
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.
Is my understanding correct that PaymentAsset
is the asset that'll be sent outgoing? Though if I think about the send vs recv flows, that means in the send flow the PaymentAsset
is BTC, but in the recv flow (receiving some other asset) it'll be that asset?
Or is it that this is the asset the penultimate hop will be receiving?
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.
I conceptualize the price oracle service as follows: it does not know on whether an asset is inbound or outbound on a channel; its primary function is to return a price. This applies regardless of whether the node is attempting to formulate a rate tick hint price or a quote accept rate tick.
Hence, it is the client's responsibility to select the SubjectAsset
and PaymentAsset
according to their needs. The TransactionType
determines the trade side the client is interested in.
The price oracle is currently called in two places:
Lines 310 to 326 in eeca76c
paymentAssetId := make([]byte, 32) | |
req := &oraclerpc.QueryRateTickRequest{ | |
TransactionType: oraclerpc.TransactionType_PURCHASE, | |
SubjectAsset: &oraclerpc.AssetSpecifier{ | |
Id: &oraclerpc.AssetSpecifier_AssetId{ | |
AssetId: subjectAssetId, | |
}, | |
}, | |
SubjectAssetMaxAmount: maxAssetAmount, | |
PaymentAsset: &oraclerpc.AssetSpecifier{ | |
Id: &oraclerpc.AssetSpecifier_AssetId{ | |
AssetId: paymentAssetId, | |
}, | |
}, | |
RateTickHint: nil, | |
} |
Lines 228 to 257 in eeca76c
paymentAssetId := make([]byte, 32) | |
// Construct the RPC rate tick hint. | |
var rateTickHint *oraclerpc.RateTick | |
if bidPrice != nil { | |
// Compute an expiry time using the default expiry delay. | |
expiryTimestamp := uint64(time.Now().Unix()) + | |
defaultRateTickExpirySeconds | |
rateTickHint = &oraclerpc.RateTick{ | |
Rate: uint64(*bidPrice), | |
ExpiryTimestamp: expiryTimestamp, | |
} | |
} | |
req := &oraclerpc.QueryRateTickRequest{ | |
TransactionType: oraclerpc.TransactionType_SALE, | |
SubjectAsset: &oraclerpc.AssetSpecifier{ | |
Id: &oraclerpc.AssetSpecifier_AssetId{ | |
AssetId: subjectAssetId, | |
}, | |
}, | |
SubjectAssetMaxAmount: assetAmount, | |
PaymentAsset: &oraclerpc.AssetSpecifier{ | |
Id: &oraclerpc.AssetSpecifier_AssetId{ | |
AssetId: paymentAssetId, | |
}, | |
}, | |
RateTickHint: rateTickHint, | |
} |
At both call sites the payment asset is BTC and it's the transaction type that varies.
I've tried to develop a basic price oracle in a generalized manner, such that it returns a price without any awareness of asset channels. I think that makes sense for light tap clients but maybe not for edge nodes?
dc12d95
to
35348fa
Compare
In this commit, we introduce a function called IsAssetBtc. This function detects whether the provided AssetSpecifier represents Bitcoin (BTC).
35348fa
to
6af97a7
Compare
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.
LGTM 🦄
} | ||
} | ||
|
||
// QueryRateTick queries the rate tick for a given transaction type, subject |
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.
I think it would be good to further lift this into a high level markdown doc (along with how to set one up, the flags, etc). Don't think we need to block merging this based on that though.
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.
I think I've mentioned this to @Liongrass .
Closes #995
This PR introduces a basic price oracle service example and ensures its compilation during our CI checks.