Implement capability for a hard fork for extended/infinite claim expiration times

kaykurokawa commented

2018-04-04 22:36:14 +02:00

(Migrated from github.com)

Based on https://github.com/lbryio/lbrycrd/pull/112, unlike this one, this PR should not hard fork mainnet or testnet. It does implement the hard forking in regtest so it can be tested downstream. Regtest sets original expiration time to 500, forks at height 800 and sets it to new expiration time 600.

Below are additions to https://github.com/lbryio/lbrycrd/pull/112:

Made #define statement to change whether we are removing claim expiration or not, currently set to not remove it.

Adjusted chainparams so mainnet and testnet does not hard fork, made regtest fork to an extended expiration time for testing purposes.

Changed the initialization of nExpirationTime in CClaimTrie to use chainparams

I adjusted claimtriebranching_no_expire to account for the fact that CClaimTrie should be using RegTesting expiration related parameters (so no need to set nExpirationTime inside the test). Also added a test that checks to see if claims that expire after the fork height still expires at the original height as expected.

Based on https://github.com/lbryio/lbrycrd/pull/112, unlike this one, this PR should not hard fork mainnet or testnet. It does implement the hard forking in regtest so it can be tested downstream. Regtest sets original expiration time to 500, forks at height 800 and sets it to new expiration time 600. Below are additions to https://github.com/lbryio/lbrycrd/pull/112: Made #define statement to change whether we are removing claim expiration or not, currently set to not remove it. Adjusted chainparams so mainnet and testnet does not hard fork, made regtest fork to an extended expiration time for testing purposes. Changed the initialization of nExpirationTime in CClaimTrie to use chainparams I adjusted claimtriebranching_no_expire to account for the fact that CClaimTrie should be using RegTesting expiration related parameters (so no need to set nExpirationTime inside the test). Also added a test that checks to see if claims that expire after the fork height still expires at the original height as expected.

lbrynaut commented

2018-04-04 23:05:28 +02:00

(Migrated from github.com)

Why an entirely new PR?

Also, I get the impression no one on this team really squashes commits, but I would urge you to squash those into 1 on top of mine.

Why an entirely new PR? Also, I get the impression no one on this team really squashes commits, but I would urge you to squash those into 1 on top of mine.

kaykurokawa commented

2018-04-05 01:36:37 +02:00

(Migrated from github.com)

Sorry, I guess I didn't have to open a new PR for this. In the other lbry repos, there's various build related things that don't run if the PR is not on a lbryio branch (if they are from someone else) due to security reasons but lbrycrd doesn't have this. Next time i'll just point you to the new commits.

I generally keep the commits unsquashed to make it easier for the reviewer. But yes, I have a bad habit of leaving them unsquashed and should squash them once its finished with review. Thanks.

Sorry, I guess I didn't have to open a new PR for this. In the other lbry repos, there's various build related things that don't run if the PR is not on a lbryio branch (if they are from someone else) due to security reasons but lbrycrd doesn't have this. Next time i'll just point you to the new commits. I generally keep the commits unsquashed to make it easier for the reviewer. But yes, I have a bad habit of leaving them unsquashed and should squash them once its finished with review. Thanks.

lbrynaut commented

2018-04-05 02:31:27 +02:00

(Migrated from github.com)

Ok, just curious. I would argue that unsquashed commits make reviewing potentially much more difficult, as it's very possible to review a large commit that is invalidated by a later commit. Reviewing the final code is all that matters.

kaykurokawa commented

2018-04-13 16:24:13 +02:00

(Migrated from github.com)

I will squash and lbrynaut will merge this

lbrynaut commented

2018-04-23 14:18:43 +02:00

(Migrated from github.com)

@kaykurokawa Hrm, the squash was not as intended -- never squash across authors. This should now be 2 commits.

kaykurokawa commented

2018-05-07 00:24:39 +02:00

(Migrated from github.com)

Rebased to master, and rebased to undo the squash into a single commit and into commits which will need to get changed so that it does exactly what we want (fork height/ whether we remove expiration altogether or not)..

kaykurokawa commented

2018-05-28 20:01:25 +02:00

(Migrated from github.com)

merged as part of https://github.com/lbryio/lbrycrd/pull/137

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Implement capability for a hard fork for extended/infinite claim expiration times #115

Pull request closed