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

multi: remove static default channel types instead select them based on lnd's verison #330

Merged
merged 4 commits into from
Dec 23, 2021

Conversation

Roasbeef
Copy link
Member

In this PR, we remove the defaults and the CLI and RPC level. We replace them with a new set of functional options that'll let us decide dynamically which features to use based on the lnd version. This lets us support older versions, while being able to activated newer features as we detect them based on what lnd offers.

Pull Request Checklist

  • LndServices minimum version has been updated if new lnd apis/fields are
    used.

@Roasbeef Roasbeef added enhancement New feature or request orders labels Dec 17, 2021
Copy link
Contributor

@positiveblue positiveblue left a comment

Choose a reason for hiding this comment

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

LGTM but I have some doubts that may be blocking

rpcserver.go Outdated Show resolved Hide resolved
order/rpc_parse.go Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Can be simplified by re-using code from lndclient. Other than that LGTM 🎉

server.go Outdated Show resolved Hide resolved
rpcserver.go Outdated Show resolved Hide resolved
rpcserver.go Show resolved Hide resolved
In this commit, we move to stop setting a default channel type on the
CLI. Instead, we want this to be selected at runtime by the daemon based
on what the lnd node can offer.
We'll use this in a follow up commit to decide what channel type we want
to use.
In this commit, we add a new set of functional options that will allow
the caller to pick an default channel type w/o forcing the main parse
function to be aware of this logic.
In this commit, we use the prior commits to allow the server to
dynamically select a channel type based on the lnd version.
@Roasbeef Roasbeef force-pushed the default-script-chan-type branch from 2c99c6f to b413ace Compare December 18, 2021 02:03
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@guggero
Copy link
Member

guggero commented Dec 23, 2021

The linter complains about the naming, will fix in separate PR.

@guggero guggero merged commit 7c2268f into master Dec 23, 2021
@guggero guggero deleted the default-script-chan-type branch December 23, 2021 10:56
@guggero guggero mentioned this pull request Dec 23, 2021
1 task
guggero added a commit to guggero/pool that referenced this pull request Apr 11, 2022
To fix an accidental market segregation for lnd 0.14.x users that didn't
set an explicit channel type in their bid orders, we roll back the
changes from lightninglabs#330.
We can re-enable this default value selection once a large number of ask
orders support the new channel type.
guggero added a commit that referenced this pull request Apr 19, 2022
positiveblue pushed a commit to positiveblue/pool that referenced this pull request Oct 11, 2022
…aper-retry

ratings: always reset the scrape time for the bos score scraper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request orders
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants