Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

feat(autopilot): enable private and minconfs=0 #698

Merged
merged 1 commit into from
Aug 21, 2018

Conversation

JimmyMow
Copy link
Member

@JimmyMow JimmyMow commented Aug 21, 2018

This PR adds private channels and minconfs=0 to our autopilot configuration.

We don't want our light client users advertising channels because they are not likely to be reliable nodes with high uptime. Fixes #681.

NOTE: This PR can only be merged after we update to LND master. These new flags aren't supported with the LND that is currently packaged and LND will shut down on start because of it at the moment. (Customized my binary path to test this works)

@JimmyMow JimmyMow requested a review from mrfelton August 21, 2018 16:27
@JimmyMow JimmyMow added this to the v0.2.2-beta milestone Aug 21, 2018
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

A couple of minor comments on the code @JimmyMow .

Also, could you add your notes about the change into the commit message, and also add a lint like:

Fix #681

into the commit message. Having the fix reference in the commit message will enable github to automatically close out that issue when this is merged, and also allows our changelog to cross reference the PR.

// Turn autopilot on
neutrinoArgs.push('--autopilot.active')
// Ensure all channels opened by autopilot will be flagged private
neutrinoArgs.push('--autopilot.private')
Copy link
Member

Choose a reason for hiding this comment

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

should we move these additional options into resources/lnd.conf? I think the only thing we need to specify dynamically here is the --autopilot.active switch.

@@ -90,6 +100,7 @@ class Neutrino extends EventEmitter {
// neutrinoArgs.push('--neutrino.connect=testnet2-btcd.zaphq.io')
}

// this.process = spawn(this.lndConfig.binaryPath, neutrinoArgs)
Copy link
Member

Choose a reason for hiding this comment

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

this commented line can go

@mrfelton
Copy link
Member

@JimmyMow this needs another update to lnd binary, over and above the one that was merged this morning?

@coveralls
Copy link

coveralls commented Aug 21, 2018

Pull Request Test Coverage Report for Build 3783

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 12.721%

Totals Coverage Status
Change from base Build 3773: 0.0%
Covered Lines: 484
Relevant Lines: 3340

💛 - Coveralls

fix(neutrino): remove unneeded comments

fix(autopilot): move private/minconf to proto
@JimmyMow JimmyMow force-pushed the feat/private-0-conf-autopilot branch from 0e741d8 to d162d45 Compare August 21, 2018 17:31
@JimmyMow
Copy link
Member Author

@mrfelton no the current LND version is fine.

Squashed commits and moved autopilot.private=1 and autopilot.minconfs=0 to the proto file

@mrfelton mrfelton merged commit 1efa299 into master Aug 21, 2018
@JimmyMow JimmyMow deleted the feat/private-0-conf-autopilot branch August 22, 2018 15:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants