lbryio / lbrycrd

The blockchain that provides the digital content namespace for the LBRY protocol
https://lbry.com
MIT License
2.57k stars 178 forks source link

Add capability to extend expiration time of pre fork claims #137

Closed kaykurokawa closed 6 years ago

kaykurokawa commented 6 years ago

Details of hard fork for all:

The fork of the mainnet will be on block 400155 (will be around noon EST 7/9/18)

The fork of the testnet will be on block 280831 (will be around noon EST 5/21/18)

Claims made on and after the target hardfork blocks will expire after 2102400 blocks (approximately 10 years assuming block time 2.5 minutes).

Claims that were made before the hardfork and have not expired will have their expiration extended by the difference between the new expiration time and the previous expiration time (456 days).

Claims that already expired will remain expired.

Details for reviewers

This is further work off of : https://github.com/lbryio/lbrycrd/pull/115 in order to add the extension of claim expiration to already existing claims ( in #115 claims made before the hard fork would still expire at the old expiration time)

While working on this, I made two adjustments to #115. A) the first was that the new expiration time took effect 1 block after the target hard fork block, and not on the target hard fork block. It would be more intuitive if the new expiration time took effect on the target hard fork block. B) I fixed the logic of calling setExpirationTime() to only be called when we are at the target hard fork block, instead of on every block. This is more intuitive and in line with the soft-fork logic that is already present for Bitcoin. This means we need to also called setExpirationTime when we load the claimtrie from disk so I did that. The combination of A) and B) fixes a bug in #115 where if you loaded the claimtrie from disk (restarted lbrycrdd) than you expiration time for the first block after restart would be the old expiration time instead of the new one even after the hard fork.

The above work is on https://github.com/lbryio/lbrycrd/pull/137/commits/2df3bb104b29f5f1cd0615efbe33ab46bc1aca53

While working on extending the expiration of existing claims, I realized that #110 had the problem where it would not properly be able to remove expirations from the expiration queue properly. This is because the function removeFromExpirationQueue() : https://github.com/lbryio/lbrycrd/blob/master/src/claimtrie.cpp#L1675 was not adjusted to consider the hard fork change in expiration time hence this line: https://github.com/lbryio/lbrycrd/blob/master/src/claimtrie.cpp#L1677 could calculate the wrong expiration time for a claim that was created before the hard fork an expires after the hard fork.

The way we extend the claim is fairly straight forward. When we encounter the hard fork block, we set the new expiration time, and we look at all the entries in the expiration queue either in the dirty queue (where entries that are not saved to disk resides) or the disk database. The entries are adjusted to have their expiration time extended by the difference between the new expiration time and the old expiration time. Effectively, this makes it look like all the claims share the same new expiration time so the issue I described is solved.

The above work on extending the expiraiton of existing claims is on b3aa62c

For the unit tests, I added a series of tests to see if supports behaved properly, and also added tests to see if the claimtrie behaved properly on the hard fork if I simulated a restart (write claimtrie to disk, clear it in memory, and than read it from disk). https://github.com/lbryio/lbrycrd/pull/137/commits/6621f50b985d49a5b5c4c09b32d871c58fa54121

The other commits should be straight forward.

kaykurokawa commented 6 years ago

It was discovered that the testnet blocks haven't been progressing (mining was turned off) so I'll need to set a new fork height for the testnet.

kaykurokawa commented 6 years ago

made fixes as reviewed by lbrynaut

kaykurokawa commented 6 years ago

Made fix as suggested by syba, Adjusted fork height on testnet to 278160 (in about an hour from now) , and I will run this on testnet. Maybe for debug logging comment by shyba, we can do another PR to perhaps improve/clean up different log messages?

shyba commented 6 years ago

@kaykurokawa yes, I agree. Created an issue for logging here.

kaykurokawa commented 6 years ago

Rebased to two commits, rebased to master