Feature/simplify api dsl #1690
Feature/simplify api dsl #1690
Conversation
apply auth and headers in public route, use specific tested handler in all specs, only use credentials in Basic auth tests
Codecov Report
@@ Coverage Diff @@
## master #1690 +/- ##
==========================================
+ Coverage 86.09% 86.28% +0.18%
==========================================
Files 151 151
Lines 11523 11701 +178
Branches 493 501 +8
==========================================
+ Hits 9921 10096 +175
- Misses 1602 1605 +3
|
|
We're currently finalizing a few features because we want to publish a new release of eclair, I'll have a look at this PR once I'm done with the release work. Don't worry if we're unable to review it this week, we will pick this up once we have some time available. |
|
Thanks, that's a really nice refactoring, things are much cleaner afterwards. I'd change a bit the categorization of the existing APIs (it's really hard because some APIs fit in multiple categories, and some don't really fit in any category). There is already a categorization that you can find in Can you update the Apart from that everything looks good in the PR. Just a couple of comments about styling:
Thanks again, I think this is a really nice refactoring and paves the way for more improvements afterwards, we will include this PR once we're finished with the next eclair release. |
|
Yes, noticed the long lines, will get used to it :). Will apply your suggestions as soon as I have some spare time on my hand. |
# Conflicts: # eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala
There was a problem hiding this comment.
Thanks, a few nits but otherwise looks good.
There is a regression though that needs to be fixed:
> alice-eclair-cli getinf
The requested resource could not be found.
> bob-eclair-cli getinf
parse error: Invalid numeric literal at line 1, column 4where alice-eclair-cli uses master and bob-eclair-cli uses your branch.
I have the same issue if I provide an invalid password, master correctly returns The supplied authentication is invalid whereas your change returns parse error: Invalid numeric literal at line 1, column 4.
You may be interested in https://github.com/t-bast/lightning-cfg to simplify E2E testing locally (this is what I used with tweaks to use your branch for Bob).
eclair-node/src/main/scala/fr/acinq/eclair/api/directives/EclairDirectives.scala
Outdated
Show resolved
Hide resolved
eclair-node/src/main/scala/fr/acinq/eclair/api/directives/ExtraDirectives.scala
Outdated
Show resolved
Hide resolved
eclair-node/src/main/scala/fr/acinq/eclair/api/serde/FormParamExtractors.scala
Outdated
Show resolved
Hide resolved
eclair-node/src/main/scala/fr/acinq/eclair/api/serde/FormParamExtractors.scala
Outdated
Show resolved
Hide resolved
…matting changes, re-added auth header in tests as it can not be isolated in handler chain order.
Since we almost always know which transactions will spend the utxos that we are watching, we can optimize the watcher to look for those instead of starting from scratch.
This fixes the `multiarg infix syntax looks like a tuple and will be deprecated` warning.
If a channel closes when we've received an UpdateFailHtlc, signed it but not yet received our peer's revocation, we need to fail the htlc upstream. That specific scenario was not correctly handled, resulting in upstream htlcs that were not failed which would force our upstream peer to close the channel.
When using anchor outputs, the commitment feerate is kept low (<10 sat/byte). When we need to force-close a channel, we must ensure the commit tx and htlc txs confirm before a given deadline, so we need to increase their feerates. This is currently done only once, at broadcast time. We use CPFP for the commit tx and RBF for the htlc txs. If publishing fails because we don't have enough utxos available, it will be retried after the next block is confirmed. Note that it's still not recommended to activate anchor outputs. More work needs to be done on this fee bumping logic and utxos management.
This command is sent by the `Peer` to its `PeerConnection`, therefore it needs to be a `RemoteType` and have its own codec.
* add tests on funding mindepth We verify that when using wumbo channels: - if we are funder we keep our regular min_depth - if we are fundee we use a greater min_depth * use lenses to simplify tags handling Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
The module is still there and can be enabled if needed.
We previously used a Set, which means you could theoretically have a feature that is both activated as `optional` and `mandatory`. We change that to be a Map `feature -> support`.
We just merged lightning/bolts@b201efe in the spec which added Bolt 3 tests, invalidating one of our tests and failing the build.
Drop mention of p2sh-segwit. We should encourage users to use bech32.
# Conflicts: # eclair-node/src/main/scala/fr/acinq/eclair/api/serde/JsonSerializers.scala
|
Thanks @tompro! |
Refactor the API handlers. Split handlers and directives in several files to make them more composable. Co-authored-by: Pierre-Marie Padiou <pm47@users.noreply.github.com> Co-authored-by: Bastien Teinturier <31281497+t-bast@users.noreply.github.com>
So here is my first try on a bit of API restructuring. Handler are now isolated from each other and grouped first by topic (of my choosing, currently just getting started with Lightning tech) and then for the public route.
Common directives like auth control and default headers are only applied on the top level entry point and free up some setup in unit tests. What I currently don't really like is the customized implicit timeout as it has to be injected into every completer but I could'nt figure out a way to create a completer that provides an implicit implicitly ;)
What do you think?