-
Notifications
You must be signed in to change notification settings - Fork 992
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use sandbox folder for txhashset validation on state sync #2685
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going to introduce a sandbox I think we should separate them more explicitly.
And we should avoid extracting the txhashset a 2nd time post-validation.
chain/src/txhashset/txhashset.rs
Outdated
@@ -115,12 +114,12 @@ impl TxHashSet { | |||
root_dir: String, | |||
commit_index: Arc<ChainStore>, | |||
header: Option<&BlockHeader>, | |||
sandbox: bool, | |||
sandbox_dir: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just pass either root_dir
or sandbox_dir
as the first param and we don't need to handle the precedence rule here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @antiochp for the review.
You have header/header_head
and header/sync_head/
reading on this open()
function, but header/
is at same level as txhashset/
so surely it doesn't exist in sandbox folder.
That's why I keep this first root_dir
param.
Could you explain more about why we need put header\
also into the sandbox
folder?
header_pmmr_h: PMMRHandle::new(
&root_dir,
HEADERHASHSET_SUBDIR,
HEADER_HEAD_SUBDIR,
false,
None,
)?,
sync_pmmr_h: PMMRHandle::new(
&root_dir,
HEADERHASHSET_SUBDIR,
SYNC_HEAD_SUBDIR,
false,
None,
)?,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, perhaps that rebuild
did the job :-)
self.rebuild_header_mmr(&Tip::from_header(&header), &mut txhashset)?;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but then what's the best to handle this header/
folder in sandbox
?
- also move to the db root
- call a
rebuild_header_mmr()
again after moving thetxhashset/
folder
Which one do you prefer? Is the rebuild_header_mmr()
a fast or slow processing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried with moving the header/ folder
, but see a lot of Corrupted storage
log which could be caused by not-atomic processing on store during txhashset_write
processing?
Switch to rebuild_header_mmr()
solution now.
And regarding the speed, the rebuilding
spent 10 seconds on my slow mac air for current main chain height. not bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh ok.
chain_data/txhashset <- txhashset.zip unzips to here
chain_data/header <- local data only, not included in the zip file (rebuilt from headers from db)
The original intent was to keep the header
dir out the txhashset
dir to make it more explicit what originated from txhashset.zip
and what was purely built locally.
Maybe we consider making the sandbox dir a couple of levels deep?
So we can have something like -
chain_data/txhashset
chain_data/header
$sandbox_dir/txhashset
$sandbox_dir/header
Would this work?
I don't see any reasons why we would not be able to copy across both the txhashset
dir and the header
dir on successful validation.
A txhashset will have file references to files under both these dirs so we'd need to be careful with ordering.
i.e. copy them across then reopen the newly updated txhashset.
Edit: rebuild_header_mmr
would work but we should not need to do this. Its not based on data from txhashset.zip
so is effectively wasted effort as we simply read the headers from the db and rebuild the header MMR a second time this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…r (2)lock txhashset when overwriting and opening and rebuild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ran "fast sync" locally and it went flawlessly.
Watched the tmp txhashet dir get used and cleaned up after validation under $TMPDIR.
👍
* forgot to clean the header folder in sandbox in #2685
* cleanup legacy "3 dot" check (#2625) * Allow to peers behind NAT to get up to preferred_max connections (#2543) Allow to peers behind NAT to get up to preffered_max connections If peer has only outbound connections it's mot likely behind NAT and we should not stop it from getting more outbound connections * Reduce usage of unwrap in p2p crate (#2627) Also change store crate a bit * Simplify (and fix) output_pos cleanup during chain compaction (#2609) * expose leaf pos iterator use it for various things in txhashset when iterating over outputs * fix * cleanup * rebuild output_pos index (and clear it out first) when compacting the chain * fixup tests * refactor to match on (output, proof) tuple * add comments to compact() to explain what is going on. * get rid of some boxing around the leaf_set iterator * cleanup * [docs] Add switch commitment documentation (#2526) * remove references to no-longer existing switch commitment hash (as switch commitments were removed in ca8447f and moved into the blinding factor of the Pedersen Commitment) * some rewording (points vs curves) and fix of small formatting issues * Add switch commitment documentation * [docs] Documents in grin repo had translated in Korean. (#2604) * Start to M/W intro translate in Korean * translate in Korean * add korean translation on intro * table_of_content.md translate in Korean. * table_of_content_KR.md finish translate in Korean, start to translate State_KR.md * add state_KR.md & commit some translation in State_KR.md * WIP stat_KR.md translation * add build_KR.md && stratum_KR.md * finish translate stratum_KR.md & table_of_content_KR.md * rename intro.KR.md to intro_KR.md * add intro_KR.md file path each language's intro.md * add Korean translation file path to stratum.md & table_of_contents.md * fix difference with grin/master * Fix TxHashSet file filter for Windows. (#2641) * Fix TxHashSet file filter for Windows. * rustfmt * Updating regexp * Adding in test case * Display the current download rate rather than the average when syncing the chain (#2633) * When syncing the chain, calculate the displayed download speed using the current rate from the most recent iteration, rather than the average download speed from the entire syncing process. * Replace the explicitly ignored variables in the pattern with an implicit ignore * remove root = true from editorconfig (#2655) * Add Medium post to intro (#2654) Spoke to @yeastplume who agreed it makes sense to add the "Grin Transactions Explained, Step-by-Step" Medium post to intro.md Open for suggestions on a better location. * add a new configure item for log_max_files (#2601) * add a new configure item for log_max_files * rustfmt * use a constant instead of multiple 32 * rustfmt * Fix the build warning of deprecated trim_right_matches (#2662) * [DOC] state.md, build.md and chain directory documents translate in Korean. (#2649) * add md files for translation. * start to translation fast-sync, code_structure. add file build_KR.md, states_KR.md * add dandelion_KR.md && simulation_KR.md for Korean translation. * add md files for translation. * start to translation fast-sync, code_structure. add file build_KR.md, states_KR.md * add dandelion_KR.md && simulation_KR.md for Korean translation. * remove some useless md files for translation. this is rearrange set up translation order. * add dot end of sentence & translate build.md in korean * remove fast-sync_KR.md * finish build_KR.md translation * finish build_KR.md translation * finish translation state_KR.md & add phrase in state.md to move other language md file * translate blocks_and_headers.md && chain_sync.md in Korean * add . in chain_sync.md , translation finished in doc/chain dir. * fix some miss typos * Api documentation fixes (#2646) * Fix the API documentation for Chain Validate (v1/chain/validate). It was documented as a POST, but it is actually a GET request, which can be seen in its handler ChainValidationHandler * Update the API V1 route list response to include the headers and merkleproof routes. Also clarify that for the chain/outputs route you must specify either byids or byheight to select outputs. * refactor(ci): reorganize CI related code (#2658) Break-down the CI related code into smaller more maintainable pieces. * Specify grin or nanogrins in API docs where applicable (#2642) * Set Content-Type in API client (#2680) * Reduce number of unwraps in chain crate (#2679) * fix: the restart of state sync doesn't work sometimes (#2687) * let check_txhashset_needed return true on abnormal case (#2684) * Reduce number of unwwaps in api crate (#2681) * Reduce number of unwwaps in api crate * Format use section * Small QoL improvements for wallet developers (#2651) * Small changes for wallet devs * Move create_nonce into Keychain trait * Replace match by map_err * Add flag to Slate to skip fee check * Fix secp dependency * Remove check_fee flag in Slate * Add Japanese edition of build.md (#2697) * catch the panic to avoid peer thread quit early (#2686) * catch the panic to avoid peer thread quit before taking the chance to ban * move catch wrapper logic down into the util crate * log the panic info * keep txhashset.rs untouched * remove a warning * [DOC] dandelion.md, simulation.md ,fast-sync.md and pruning.md documents translate in Korean. (#2678) * Show response code in API client error message (#2683) It's hard to investigate what happens when an API client error is printed out * Add some better logging for get_outputs_by_id failure states (#2705) * Switch commitment doc fixes (#2645) Fix some typos and remove the use of parentheses in a couple of places to make the reading flow a bit better. * docs: update/add new README.md badges (#2708) Replace existing badges with SVG counterparts and add a bunch of new ones. * Update intro.md (#2702) Add mention of censoring attack prevented by range proofs * use sandbox folder for txhashset validation on state sync (#2685) * use sandbox folder for txhashset validation on state sync * rustfmt * use temp directory as the sandbox instead actual db_root txhashset dir * rustfmt * move txhashset overwrite to the end of full validation * fix travis-ci test * rustfmt * fix: hashset have 2 folders including txhashset and header * rustfmt * (1)switch to rebuild_header_mmr instead of copy the sandbox header mmr (2)lock txhashset when overwriting and opening and rebuild * minor improve on sandbox_dir * add Japanese edition of state.md (#2703) * Attempt to fix broken TUI locale (#2713) Can confirm that on the same machine 1.0.2 TUI looks great and is broken on the current master. Bump of `cursive` version fixed it for me. Fixes #2676 * clean the header folder in sandbox (#2716) * forgot to clean the header folder in sandbox in #2685 * Reduce number of unwraps in servers crate (#2707) It doesn't include stratum server which is sufficiently changed in 1.1 branch and adapters, which is big enough for a separate PR. * rustfmt * change version to beta
An improvement on the txhashset validation procedure: use a sandbox folder for new downloaded txhashset files and the validation is based on sandbox folder firstly. The local txhashset will be overwritten ONLY when validation is ok.
The sandbox folder will be cleaned immediately no matter what's the validation result.