Skip to content

Fix expiration fork usage#296

Merged
BrannonKing merged 3 commits intomasterfrom
expiration_fix
Jul 22, 2019
Merged

Fix expiration fork usage#296
BrannonKing merged 3 commits intomasterfrom
expiration_fix

Conversation

@bvbfan
Copy link
Copy Markdown
Collaborator

@bvbfan bvbfan commented Jul 10, 2019

Signed-off-by: Anthony Fieroni [email protected]

Fixes #287

Signed-off-by: Anthony Fieroni <[email protected]>
{
ClaimTrieChainFixture fixture;
fixture.setExpirationTime(3);
fixture.setExpirationForkHeight(800, 3, 800);
Copy link
Copy Markdown
Member

@BrannonKing BrannonKing Jul 16, 2019

Choose a reason for hiding this comment

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

I don't think this is right. Aren't those 800s supposed to be 140, the current height? What was the old behavior? I don't see that this test goes all the way to 800 increments, so I doubt it's hitting the danger zone.

Copy link
Copy Markdown
Collaborator Author

@bvbfan bvbfan Jul 16, 2019

Choose a reason for hiding this comment

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

In ClaimTrieChainFixture we don't change the consensus, so setExpirationTime was supposed to change only expiration time, since we don't have such functionality it should be

fixture.setExpirationForkHeight(consensus.nExtendedClaimExpirationForkHeight, 3, consensus.nExtendedClaimExpirationTime)

@lbry-bot lbry-bot assigned bvbfan and unassigned bvbfan Jul 16, 2019
@BrannonKing BrannonKing merged commit b1aa5e0 into master Jul 22, 2019
@BrannonKing BrannonKing deleted the expiration_fix branch July 22, 2019 20:27
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.

expiration fork isn't set in miner.cpp

2 participants