-
Notifications
You must be signed in to change notification settings - Fork 95
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
itest: Extend custom channel liquidity itest for RFQ HTLC tracking #906
itest: Extend custom channel liquidity itest for RFQ HTLC tracking #906
Conversation
6b56147
to
f055328
Compare
7cb58a5
to
d82cb9d
Compare
@GeorgeTsagk do I only need to review the last three commits? |
f055328
to
e52a8c5
Compare
@ffranr this was just rebased, it's just one commit now (if you skip the temp one) |
b0c1550
to
d48feca
Compare
9ef04d6
to
82d567c
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.
Needs a couple of updates still, but definitely the better approach!
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 reviewed the latest commits. I think we just need to address Oli's comments here.
82d567c
to
262846f
Compare
2a8b4ad
to
83801f5
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 pending dependent PRs.
d4505dd
to
868e57c
Compare
83801f5
to
acbc6b5
Compare
acbc6b5
to
0381304
Compare
0381304
to
99ab0ce
Compare
Will wait for base PR to merge, then will update |
@@ -882,44 +896,61 @@ func payInvoiceWithAssets(t *testing.T, payer, rfqPeer *HarnessNode, | |||
sendReq.MaxShardSizeMsat = 80_000_000 | |||
} | |||
|
|||
var rfqBytes []byte | |||
manualRfq.WhenSome(func(i rfqmsg.ID) { | |||
copy(rfqBytes, i[:]) |
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.
copy()
is tricky if the target slice is nil or empty, nothing happens. So you need to do rfqBytes = make([]byte, len(i[:]))
first.
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.
Nice catch! Interestingly enough the tests did not fail as the daemon would automatically acquire fresh quotes for the payment. Specifying an rfqID here was a effectively a noop.
The reason this wasn't caught by the next test case (which requires you use a specific rfq scid and none else) is because it is enforced by the receiver, while crafting the invoice.
99ab0ce
to
aff79f2
Compare
aff79f2
to
73bbd6a
Compare
82c8511
into
lightninglabs:update-to-lnd-18-4
Descriptions
Adds an extra edge case to the custom channels itest which verifieds that the rfq htlc tracking mechanism works as expected.
Related tapd PR: lightninglabs/taproot-assets#1186