Skip to content
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 spv_refundhtlcall #686

Merged
merged 7 commits into from
Oct 22, 2021
Merged

Add spv_refundhtlcall #686

merged 7 commits into from
Oct 22, 2021

Conversation

Bushstar
Copy link
Member

@Bushstar Bushstar commented Aug 18, 2021

Adds a call to refund all HTLCs in the wallet for which the correct private key is known and has enough confirmations.

Fixes issue where only one HTLC output in a TX could be seen or spent.

Fixes issue where HTLC refund will try to spend output without the required confirmations.

Only shows sendmessage RPC response text when there's an error on send, otherwise this will always display that an unknown error has occured.

Comment on lines +888 to +892
// Sort by Bitcoin address
std::sort(htlcTransactions.begin(), htlcTransactions.end(), [](const std::pair<BRTransaction*, size_t>& lhs, const std::pair<BRTransaction*, size_t>& rhs){
return strcmp(lhs.first->outputs[lhs.second].address, rhs.first->outputs[rhs.second].address) < 0;
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Why sorting? And why not in BRListHTLCReceived ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Will still need to be sorted in BRListHTLCReceived in the same way. I was thinking of adding later an option for the user to specify sort order, like TXID, time, address and so on. I could have used all three in testing but address order sufficed.

bvbfan
bvbfan previously approved these changes Aug 24, 2021
CWallet* const pwallet = GetWallet(request);

RPCHelpMan{"spv_refundhtlcall",
"\nGets all HTLC contracts stored in wallet and creates refunds transactions for a that have expired\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

The help message reads not correct.

@prasannavl prasannavl changed the base branch from master to 2.0.x October 22, 2021 13:09
@prasannavl prasannavl dismissed bvbfan’s stale review October 22, 2021 13:09

The base branch was changed.

prasannavl
prasannavl previously approved these changes Oct 22, 2021
@prasannavl prasannavl merged commit 20b2549 into 2.0.x Oct 22, 2021
@prasannavl prasannavl deleted the spv-refundhtlcall branch October 22, 2021 15:23
prasannavl added a commit that referenced this pull request Oct 30, 2021
* Add spv_refundhtlcall

* sendmessage only populated on failure

* sendmessage only populated on error

* Set block heights in test to meet refund requirement

* Fix lints

* Update help test

Co-authored-by: Prasanna Loganathar <[email protected]>
prasannavl added a commit that referenced this pull request Oct 30, 2021
* Add spv_refundhtlcall

* sendmessage only populated on failure

* sendmessage only populated on error

* Set block heights in test to meet refund requirement

* Fix lints

* Update help test

Co-authored-by: Prasanna Loganathar <[email protected]>
prasannavl added a commit that referenced this pull request Oct 30, 2021
* Add spv_refundhtlcall

* sendmessage only populated on failure

* sendmessage only populated on error

* Set block heights in test to meet refund requirement

* Fix lints

* Update help test

Co-authored-by: Prasanna Loganathar <[email protected]>
prasannavl added a commit that referenced this pull request Oct 30, 2021
* Add spv_refundhtlcall

* sendmessage only populated on failure

* sendmessage only populated on error

* Set block heights in test to meet refund requirement

* Fix lints

* Update help test

Co-authored-by: Prasanna Loganathar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants