Skip to content
This repository has been archived by the owner on Jul 15, 2020. It is now read-only.

Store out-of-order blocks from local devices #12

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

luke-jr
Copy link

@luke-jr luke-jr commented Jan 29, 2019

No description provided.

@luke-jr
Copy link
Author

luke-jr commented Jan 30, 2019

Fixed a number of issues, and moved main out-of-order-blocks code to another file.

return ooob_db;
}

bool StoreOoOBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock) EXCLUSIVE_LOCKS_REQUIRED(cs_main)

Choose a reason for hiding this comment

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

It'd be nice to put a CheckBlock in here first.

Copy link
Author

Choose a reason for hiding this comment

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

Put it at the caller, since that's where it's deciding whether or not to do the storing.

@@ -3431,7 +3431,7 @@ bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CVali
return true;
}

bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock, bool fForceProcessing, bool *fNewBlock)
bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr<const CBlock> pblock, bool fForceProcessing, bool *fNewBlock, const CDiskBlockPos *diskpos, const bool do_ooob)

Choose a reason for hiding this comment

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

Instead of refactoring PNB to pass the diskpos through, I think it would be more maintainable if you just call AcceptBlock from outoforder.cpp.

Copy link
Author

Choose a reason for hiding this comment

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

AcceptBlock is not exposed outside of validation.cpp, and calling it directly would entail rewriting the rest of ProcessNewBlock too - which will make rebasing more complex, as any changes there would need to be ported over.

@bg002h
Copy link

bg002h commented Jan 31, 2019

I tested this by turning off my satellite connection to skip blocks 560822 through and including 560825. I kept bitcoind running. I then turned blocksat-rx back on and Luke's code kept blocks 560826- to the tip cached while I waited for the satellite to broadcast my missing blocks. Once I got block 560825 to close the gap in my chain, my node caught right up to the tip in short order.

@bg002h
Copy link

bg002h commented Apr 30, 2019

I've been running this code since the day Luke wrote it. No issues to report. Many successful recoveries from a temporarily unavailable connection (like every Friday when pool guy comes and parks his truck in front of my satellite dish).

@grubles
Copy link

grubles commented May 17, 2019

I'd like to bump this and get it merged so it will be easier for Blockstream Satellite users to build. Without this patch, those users' nodes may become stuck.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants