Service nodes restore only 1 rollback bug #206
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mizuki-hikaru
approved these changes
Sep 4, 2018
| @@ -107,7 +107,10 @@ namespace service_nodes | |||
| } | |||
| } | |||
|
|
|||
| m_rollback_events.push_back(std::unique_ptr<rollback_event>(new prevent_rollback(current_height))); | |||
There was a problem hiding this comment.
Actually, because we are scanning from the beginning of the list now (and not from 30 days any more), then it is fine if we just remove this line entirely. No need to prevent rollbacks anymore.
There was a problem hiding this comment.
I also didn't see this change was a push_front. That's also fine.
There was a problem hiding this comment.
Just realised block added generic gets called as part of init so the prevent rollback case is covered there.
| @@ -505,6 +508,7 @@ namespace service_nodes | |||
| { | |||
| m_rollback_events.pop_front(); | |||
| } | |||
| m_rollback_events.push_front(std::unique_ptr<rollback_event>(new prevent_rollback(cull_height))); | |||
There was a problem hiding this comment.
No need to do this. ROLLBACK_EVENT_EXPIRATION_BLOCKS must be greater than reorg safety buffer (at least, it should be). We assume that there will never be a reorg greater than rollback event expiration blocks.
There was a problem hiding this comment.
(please do take this part out though, it's not logical)
There was a problem hiding this comment.
We don't want to guard our code? Reorgs greater than 20 have been happening in testnet. Mainnet sure, might not happen. If it triggers, then everyones chain halts.
There was a problem hiding this comment.
Ah, I didn't see this was a push_front; I thought it was a push_back. This is fine.
| @@ -975,10 +977,10 @@ namespace service_nodes | |||
| i->m_block_height = from.m_block_height; | |||
| i->type = rollback_event::prevent_type; | |||
| m_rollback_events.push_back(std::unique_ptr<rollback_event>(i)); | |||
| break; | |||
There was a problem hiding this comment.
Removing these breaks is sufficient. Looks like this used to be a switch
mizuki-hikaru
approved these changes
Sep 4, 2018
It gets called in on block added generic anyway.
Doy-lee
added a commit
to Doy-lee/loki
that referenced
this pull request
Sep 5, 2018
* Fix restore 1 rollback event, ensure prevent rollback is always added * Remove adding prevent_rollback event at init It gets called in on block added generic anyway.
Doy-lee
added a commit
to Doy-lee/loki
that referenced
this pull request
Sep 7, 2018
* core: submit uptime proof immediately after registering * Increase visibility of autostaking prompts * quorum_cop: changed uptime proof prune timeout to 2 hours 10 minutes * cleanup: removed scope limiting block * check_tx_inputs: fix deregister double spend test to include deregisters from other heights * config: new testnet network id, genesis tx, and version bump * wallet2: fix testnet wallet blockheight approximation * Fix change in address format in RPC which broke parsing and pooling contributors (oxen-io#184) * Fix service node endpoints for RPC to also use stdout (oxen-io#185) * fixed some further rct core tests (oxen-io#180) * Fix service node state by calling detached hooks on failure to switch to alt chain (oxen-io#188) * fixed block verification core tests (oxen-io#186) * fixed block verification core tests * core tests: removed gen_block_miner_tx_out_is_small which is only relevant to hardfork version 1 * Don't consider expired deregistrations when filling block template * Add unit tests for getting staking requirement (oxen-io#191) * First service node test (oxen-io#190) * core_tests: added service node tests * core_tests: check balance after registration tx * Fix underflow for popping rollback events (oxen-io#189) * Move deregistration age check into check_tx_inputs * Zero initialise rct_signatures member txnFee is a uint64_t and has uninit values * Enforce that deregisters must be 0 fee since we skip checks * Add unit tests for vote validation (oxen-io#193) * Add unit tests for deregistration validation (oxen-io#194) * Mainnet checkpoint 86535, testnet 3591, 4166 * Bump version number * Add print_sr for getting staking requirement (oxen-io#198) * Misc bugfixes (oxen-io#203) * removed unnecessary cast to double during txfee+coinbase calculation * simplewallet: increased autostaking interval from 2 minutes to 40 * Fix casting issues from uint to int (oxen-io#204) * core_tests: check service node registration and expiration (oxen-io#195) * core_tests: check service node registration and deregistration * core_tests for service nodes: - include service nodes rewards when calculating account's balance - check that service nodes rewards have been received * fixed namespace error; reduced the scope of staking requirement constants * On blockchain inc/dec mark deregisters relayble based on age (oxen-io#201) * Service nodes restore only 1 rollback bug (oxen-io#206) * Fix restore 1 rollback event, ensure prevent rollback is always added * Remove adding prevent_rollback event at init It gets called in on block added generic anyway. * Log db exception, fix relation operators for vote/deregister lifetime (oxen-io#207) * Filter relayable deregisters w/ check_tx_inputs instead of blockchain callbacks * Bump version to 0.3.7-beta * fix build with GCC 8.1.0 (oxen-io#211) * Add temp hardfork rule in testnet for deregister lifetimes (oxen-io#210) * Update testnet, remove testnet forks, remove checkpoints, update blockheight estimate (oxen-io#212) * Don't ban peers for a bad vote, just drop their connection (oxen-io#213) * Update to version 0.3.0 release candidate (oxen-io#215)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Early break in the for loop causes only 1 event to be loaded. Seems like the prevent_rollback should be push_front onto the rollback list and all the rollback events are loaded after it.
Also ensure that the prevent rollback is present when culling the event list.