Skip to content

Add option --from-blockheight - #990

Closed
lvaccaro wants to merge 2 commits into
ElementsProject:masterfrom
lvaccaro:option-blockheight
Closed

Add option --from-blockheight#990
lvaccaro wants to merge 2 commits into
ElementsProject:masterfrom
lvaccaro:option-blockheight

Conversation

@lvaccaro

Copy link
Copy Markdown
Contributor

Add option --from-blockheight for lightningd to define the first block to add at startup time.
If not specify, or set to default value -1, lightningd uses the current strategy.
In according to setup_topology(), block download starts one before the block selected in --from-blockheight.

Help:

$ lightingd --help
...
--max-locktime-blocks <arg>    Maximum blocks a peer can lock up our funds (default: 432)
--from-blockheight <arg>       Add blocks from blockchain height (default: -1)
--anchor-onchain <arg>         Blocks before we give up on pending anchor transaction (default: 100)
...

Example:

$ lightingd --from-blockheight 500000
...
2018-02-13T16:02:05.745Z lightningd(28362): Adding block 499999: 0000000000000000007962066dcd6675830883516bcf40047d42740a85eb2919
...
2018-02-13T16:02:05.944Z lightningd(28362): Adding block 500000: 00000000000000000024fb37364cbf81fd49cc2d51c09c75c35433c3a1945d04
2018-02-13T16:02:40.595Z lightningd(28442): Adding block 500001: 0000000000000000005c9959b3216f8640f94ec96edea69fe12ad7dee8b74e92

Resolve the following issue:

  1. send satoshi in a lightning wallet created by lightning-cli newaddr (for example at height 500000)
  2. check incoming transaction: lightning-cli listfunds show outputs available to spent
  3. wait 102 blocks
  4. restart lightningd (in log: first block added is 100-1 blocks before the current blockchain height = 500001)
  5. lightning-cli listfunds shows no outputs

Start lightningd with the option --from-blockheight 500000 resolves this issue and listfunds shows outputs available to spent.

@rustyrussell

Copy link
Copy Markdown
Contributor

Hi! Thanks for this, could you also look at #940 which has feedback from @cdecker and me?

I think the right fix for this is to adjust the low watermark based on incoming transactions; @cdecker was looking at saving some of this information to the db, but perhaps we should just put a wallet low-watermark in there for now?

Comment thread lightningd/lightningd.h
u32 locktime_max;

/* Add block from blockheight (-1 to use default chaintopology strategy). */
s32 from_blockheight;

@cdecker cdecker Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is probably a y43k bug in the making 😉

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Do you suggest to use s64? or use a different default value?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just kidding :-)

Comment thread lightningd/lightningd.c

if (ld->config.from_blockheight > -1){
// Get_init_blockhash() go back of 100 blocks
peer_first_blocknum = ld->config.from_blockheight + 100;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you sure that we want to start 100 blocks after the specified value?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

get_init_blockhash starts 100 blocks behind the selected block. If it is more safe to always consider 100 blocks before the selected input block ( so the first "Adding block", in the log, starts 100 blocks before the specified blockheight in the option), I will remove this sum.

@cdecker cdecker Feb 14, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The + suggestes to me that if I specify height 500 this would start at 600 which is rather uexpected to me.

EDIT: Oh I see, it's compensating the -= 100 that we do in get_init_blockhash, I'd rather pull that subtraction out, instead of trying to compensate here.

@rustyrussell

Copy link
Copy Markdown
Contributor

Um, we're getting distracted :) The real fix is to put wallet utxss into the db; we can do the minimal thing if we want (just keep the low watermark in db vars) but I wanted to know if @cdecker had other plans to put wallet utxos into the db?

@cdecker

cdecker commented Feb 14, 2018

Copy link
Copy Markdown
Member

Well we have our own UTXOs (except the onchaind tracked ones) in the outputs table. We can of course hook into the onchaind watches to add those as well. Any low water mark would need to be below the onchaind tracking anyway so I think I'll just do that. Alternatively we can track all P2WSH outputs as well, so we can feed txout lookups from a table instead of calling out to bitcoind, but I'm not sure yet whether that overhead is worth it.

rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Feb 15, 2018
With fallback depending on chainparams: this means the first upgrade
will be slow, but after that it'll be fast.

Fixes: ElementsProject#990
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Feb 15, 2018
As described by @lvaccaro in ElementsProject#990.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@lvaccaro lvaccaro closed this Feb 15, 2018
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Feb 15, 2018
With fallback depending on chainparams: this means the first upgrade
will be slow, but after that it'll be fast.

Fixes: ElementsProject#990
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell added a commit to rustyrussell/lightning that referenced this pull request Feb 15, 2018
As described by @lvaccaro in ElementsProject#990.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker pushed a commit that referenced this pull request Feb 16, 2018
With fallback depending on chainparams: this means the first upgrade
will be slow, but after that it'll be fast.

Fixes: #990
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
cdecker pushed a commit that referenced this pull request Feb 16, 2018
As described by @lvaccaro in #990.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants