Improve Confirm docs
#1900
Improve Confirm docs
#1900
Conversation
|
Draft for now, I want to do another pass trying to highlight any pitfalls. Moreover, I'm thinking of adding a usage basic example. |
lightning/src/chain/mod.rs
Outdated
| /// block. | ||
| /// - Dependent transactions within the same block must be given in topological order, possibly in | ||
| /// separate calls. | ||
| /// - Unconfirmed transactions must be given after the original confirmations and before any | ||
| /// reconfirmation. | ||
| /// - Reconfirmed transactions need to be explicitly unconfirmed before they are reconfirmed in the |
There was a problem hiding this comment.
We should maybe just rephrase all of this as "everything has to be in topological order"? I guess we want to let someone connect uncorrelated transactions in the same block in any order :/. We at least need to mention that all unconfirms have to happen before any confirmations.
There was a problem hiding this comment.
I tend to disagree: I think we should not resort to terminology such as "topological" that may or may not known to the users and might be ambiguous or unclear to some. Rather, we should try to make things as explicit as possible, even if the description can get redundant at times: redundancy can be a good thing in writing.
The fact that we need to unconfirm all before reconfirming any is one of the reasons why it's still in draft, as I wanted to make sure clarity is improved here. Now pushed a fixup regarding that.
Codecov ReportBase: 90.71% // Head: 91.19% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1900 +/- ##
==========================================
+ Coverage 90.71% 91.19% +0.48%
==========================================
Files 91 91
Lines 48400 54891 +6491
Branches 48400 54891 +6491
==========================================
+ Hits 43905 50060 +6155
- Misses 4495 4831 +336
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
Marking this ready-for-review. I think it it would be nice to have a more concrete usage example under https://lightningdevkit.org/blockchain_data/confirmed_transactions/ eventually, though. |
| /// - All unconfirmed transactions must be given consecutively after the original confirmations | ||
| /// and before *any* reconfirmations. | ||
| /// - Any reconfirmed transactions need to be explicitly unconfirmed before they are reconfirmed | ||
| /// in regard to the new block. |
There was a problem hiding this comment.
| /// in regard to the new block. | |
| /// in the new block. |
There was a problem hiding this comment.
Mh, likewise I added that to somewhat separate a) the event of a transaction being confirmed in a block on chain and b) us getting notice of that and calling transactions_confirmed. The former is literally happening in the new block, while the latter is happening in regard to the former, no?
There was a problem hiding this comment.
I guess? I dunno it just read funny to me, it doesn't really matter.
tnull
force-pushed
the
2022-12-improve-confirm-docs
branch
from
7089741 to
dfe468a
Compare
tnull
force-pushed
the
2022-12-improve-confirm-docs
branch
from
dfe468a to
9de45ce
Compare
|
Squashed without further changes. |
Fixes #1188.
We improve the wording of the
Confirmdocs to improve clarity.