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

config: Experimental support for Litecoin/simnet #1865

Merged
merged 1 commit into from
Apr 19, 2019

Conversation

offerm
Copy link
Contributor

@offerm offerm commented Sep 7, 2018

This PR adds the configuration needed to run LND with Litecoin on simnet. The change is minimal and has no impact for users that don't enable this mode. When using this configuration, the user is being warned that this mode is not officially supported.

Fixes #1830.

@halseth halseth added the needs testing PR hasn't yet been actively tested on testnet/mainnet label Sep 8, 2018
halseth
halseth previously approved these changes Sep 8, 2018
Copy link
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

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

utACK, should be safe to add.

lnd.go Outdated
@@ -134,6 +134,10 @@ func lndMain() error {
strings.Title(registeredChains.PrimaryChain().String()),
network,
)
if cfg.Litecoin.SimNet {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed, no risk running on simnet anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added that here just to inform the user that he can't expect community and LND's team help if encountered problems related to Litecoin and SimNet.

Should I take it out?

Let me know @halseth please.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would say take it out.

@halseth halseth added the first pass review done PR has had first pass of review, needs more tho label Sep 8, 2018
This PR adds the configuration needed to run LND with Litecoin on simnet. The change is minimal and has no impact for users that don't enable this mode. When using this configuration, the user is being warned that this mode is not officially supported.
@offerm
Copy link
Contributor Author

offerm commented Sep 14, 2018

@halseth took out per your suggestion.

Anything else?

@cfromknecht
Copy link
Contributor

The simnet params for ltcd should be updated as well: https://github.com/ltcsuite/ltcd/blob/master/chaincfg/params.go#L492

Currently they are just copied from bitcoin simnet. Some things that would need to change:

  • SubsidyReductionInterval,TargetTimespan, TargetTimePerBlock, MinDiffReductionTime should be made equal to the mainnet values
  • MinDiffReductionTime should be set to the testnet value
  • HDCoinType should be 1

@offerm
Copy link
Contributor Author

offerm commented Sep 17, 2018

The simnet params for ltcd should be updated as well: https://github.com/ltcsuite/ltcd/blob/master/chaincfg/params.go#L492

Currently they are just copied from bitcoin simnet. Some things that would need to change:

  • SubsidyReductionInterval,TargetTimespan, TargetTimePerBlock, MinDiffReductionTime should be made equal to the mainnet values
  • MinDiffReductionTime should be set to the testnet value
  • HDCoinType should be 1

@cfromknecht do you suggest to open a PR against ltcsuite/ltcd and wait for a change? or maybe fix these values on LND side once SimNetParams for litecoin selected?

Also, since this is a private network why these parameters are so important?
Any idea why these did change on ltcsuite/ltcd ?

please guide me.

@cfromknecht
Copy link
Contributor

@offerm I would definitely welcome a PR to ltcd making those param changes, this is the primary reason ltc simnet was never exposed in lnd.

or maybe fix these values on LND side once SimNetParams for litecoin selected?

I'd prefer not to hack this in, the changes to ltcd are pretty minimal and worth doing properly IMO. FWIW, the ltcd backend would then have different parameters from LND.

@Roasbeef
Copy link
Member

FWIW, you're aware that litecoind supports regtest right?

@dannypaz
Copy link
Contributor

dannypaz commented Oct 2, 2018

@Roasbeef litecoind regtest is supported, but LND does not

lnd/config.go

Line 549 in e6925cc

str := "%s: regnet mode for litecoin not currently supported"
. It looks like the same changes would have to be made for regtest as well

@offerm
Copy link
Contributor Author

offerm commented Oct 8, 2018

@Roasbeef @cfromknecht LTCD is supporting simnet already (even if using wrong parameters).
The only thing which is missing is this small configuration to LND.

@cfromknecht two questions related to these parameters which should change on LTCD:
a. can you please let me know how do you know that these parameters should change and what their value should be? I would like to get educated before opening such PR.
b. Why to mix the current PR to LND with an unrelated one for LTCD?

The simnet params for ltcd should be updated as well: https://github.com/ltcsuite/ltcd/blob/master/chaincfg/params.go#L492
Currently they are just copied from bitcoin simnet. Some things that would need to change:

  • SubsidyReductionInterval,TargetTimespan, TargetTimePerBlock, MinDiffReductionTime should be made equal to the mainnet values
  • MinDiffReductionTime should be set to the testnet value
  • HDCoinType should be 1

@cfromknecht do you suggest to open a PR against ltcsuite/ltcd and wait for a change? or maybe fix these values on LND side once SimNetParams for litecoin selected?

Also, since this is a private network why these parameters are so important?
Any idea why these did change on ltcsuite/ltcd ?

please guide me.

@dannypaz
Copy link
Contributor

dannypaz commented Oct 8, 2018

@offerm Hoping that this answer helps you

A) litecoin/bitcoin are very similar in ways, but also have their own protocol level differences (e.g. transaction times or supply). You'll have to look at differences between the two, to know which settings need to be changed. I've found that simply searching for bitcoin chain params or litecoin chain params results provide good information on the what/why/how of some of these settings.

B) Its probably meant as a quality issue. You will want the LTCD changes merged before making the change to LND or we wont be running a 'correct' simnet.

@offerm
Copy link
Contributor Author

offerm commented Oct 14, 2018

@cfromknecht @halseth per your request I created
ltcsuite/ltcd#12

Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM 🌈

@cfromknecht cfromknecht merged commit ab4a675 into lightningnetwork:master Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first pass review done PR has had first pass of review, needs more tho needs testing PR hasn't yet been actively tested on testnet/mainnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] LND support for Litecoin on Simnet
5 participants