-
Notifications
You must be signed in to change notification settings - Fork 118
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
Add simple asset autoloop #886
base: master
Are you sure you want to change the base?
Conversation
39fbaf6
to
30bf919
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.
Great work as usual @sputn1ck. In my first pass I've left a few suggestions, comments and nits.
liquidity/liquidity.go
Outdated
@@ -641,6 +641,7 @@ func (m *Manager) dispatchBestEasyAutoloopSwap(ctx context.Context) error { | |||
|
|||
suggestion, err := builder.buildSwap( | |||
ctx, channel.PubKeyBytes, outgoing, swapAmt, easyParams, | |||
nil, |
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.
then we wouldn't have to this. Otherwise, maybe pass in a name here like assetSwapInfo := nil
.
err := json.Unmarshal(channel.CustomChannelData, &assetData) | ||
if err != nil { | ||
log.Errorf("Error unmarshalling custom channel data: %v", err) | ||
return nil |
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 we should return the error here.
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.
No, custom channel data might not be tap related so this unmarshalling might fail. We deliberately return no error in this function
assets/client.go
Outdated
AssetIdStr: assetId, | ||
}, | ||
}, | ||
PaymentMaxAmt: uint64(satMinAmt), |
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 leave a comment here why the minimum swap amount has to match the maximum amount the edge node has to pay?
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 renamed the var. this is just to signal the edge node what the maximum payment amount would be, but essentially unused as we just want to gauge the price. This whole function will be changed once lightninglabs/taproot-assets#1397 lands
|
||
acceptedRes := rfq.GetAcceptedQuote() | ||
if acceptedRes == nil { | ||
return 0, fmt.Errorf("no accepted quote") |
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.
nit: "quote wasn't accepted", maybe with details?
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.
This actually isn't "quote wasn't accepted" as that would be rfq.GetRejectedQuote()
. This would be an internal error in the grpc call so pretty unlikely and a check for the .proto oneof
message type.
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.
func (x *AddAssetSellOrderResponse) GetAcceptedQuote() *PeerAcceptedSellQuote {
if x, ok := x.GetResponse().(*AddAssetSellOrderResponse_AcceptedQuote); ok {
return x.AcceptedQuote
}
return nil
}
func (x *AddAssetSellOrderResponse) GetInvalidQuote() *InvalidQuoteResponse {
if x, ok := x.GetResponse().(*AddAssetSellOrderResponse_InvalidQuote); ok {
return x.InvalidQuote
}
return nil
}
func (x *AddAssetSellOrderResponse) GetRejectedQuote() *RejectedQuoteResponse {
if x, ok := x.GetResponse().(*AddAssetSellOrderResponse_RejectedQuote); ok {
return x.RejectedQuote
}
return nil
}
These are the 3 possibilities
60c3a87
to
8c6e77a
Compare
// gauge a price. | ||
rfqExpiry := time.Now().Add(time.Minute).Unix() | ||
|
||
// First we'll rfq a random peer for the asset. |
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.
How does the random peer selection work? Do we have to specify a peer here? In regtest I see LQDY: easy autoloop asset failed: id: 6edf1fbd38d326fa6272d56d82f3bfa043e2988fab78d82ddf2dbc64e3f7b72b, err: rpc error: code = Unknown desc = sell order peer must be specified
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 rpc documentation it read optional
for the peer pubkey. I added using the pubkey
8c6e77a
to
e3dc42f
Compare
e3dc42f
to
5f72083
Compare
5f72083
to
4a0b1eb
Compare
This PR adds a simple asset autoloop mode by reusing the easy autoloop runloop.
Testing instructions:
In order to test you need a docker-regtest setup with an asset channel.
Afterwards set up your loop with the extra flags:
You then need to activate your asset autoloop with
Then you can run
./loop-debug forceautoloop
this should dispatch a loop outAI generated summary:
This pull request introduces significant changes to the
assets/client.go
andliquidity/liquidity.go
files, adding new functionality for handling asset prices and improving the overall asset management. The most important changes include the addition of a new method to get asset prices, modifications to the client configuration to include an asset client, and updates to the liquidity manager to support easy autoloop functionality for assets.Asset Management Enhancements:
assets/client.go
: Added a new methodGetAssetPrice
to retrieve the price of an asset in satoshis using the RFQ process. Also, added a helper functiongetSatsFromAssetAmt
to convert asset amounts to satoshis.assets/client_test.go
: Added tests for the newgetSatsFromAssetAmt
function to ensure correct conversion of asset amounts to satoshis.Client Configuration Updates:
client.go
: Updated theClient
struct and related methods to useAssetClient
instead ofassetClient
, ensuring consistency and proper initialization. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]config.go
: AddedAssetClient
to theclientConfig
struct to support asset-related operations.Liquidity Manager Enhancements:
liquidity/liquidity.go
: Introduced new functionality for easy autoloop operations for assets, including methods to handle asset prices and dispatch swaps based on asset balances. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]These changes collectively enhance the system's ability to manage and utilize assets more effectively, providing better support for asset-related operations and improving the overall functionality of the liquidity manager.