Implement capability for a hard fork for extended/infinite claim expiration times #115
Implement capability for a hard fork for extended/infinite claim expiration times #115kaykurokawa wants to merge 4 commits intomasterfrom
Conversation
|
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. |
|
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. |
|
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. |
|
I will squash and lbrynaut will merge this |
kaykurokawa
force-pushed
the
claim-expiration
branch
2 times, most recently
from
90d1a0a to
dd1ad3a
Compare
|
@kaykurokawa Hrm, the squash was not as intended -- never squash across authors. This should now be 2 commits. |
kaykurokawa
force-pushed
the
claim-expiration
branch
from
dd1ad3a to
ef70e67
Compare
…. Make the if logic a bit easier to understand
…on forking, change regtest to expire and fork quicker. Use chainparams to set initial nExpirationTime in CClaimTrie
… claimtriebranching_tests, fixed white space. Use regetst parameters for claimtriebrancing_no_expire tests. Added additional test to make sure claim that expires after fork, still expires as expected
kaykurokawa
force-pushed
the
claim-expiration
branch
from
ef70e67 to
a54b6dc
Compare
|
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).. |
|
merged as part of #137 |
2 participants
Based on #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 #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.