Bugfix: Can't generate proof for transactions on the last signed block number #1795
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
Alenar
force-pushed
the
djo/1785/last_tx_not_proved
branch
from
9b71577 to
a1ee335
Compare
Alenar
force-pushed
the
djo/1785/last_tx_not_proved
branch
from
a1ee335 to
afef448
Compare
The previous, private, `replace` is renamed to `replace_unchecked`. The new replace call `replace_unchecked` but it check that the replaced key exist and that the value is the same. Co-authored-by: Sébastien Fauvel <sfauvel@users.noreply.github.com>
By using shared methods to construct boilerplate.
Before it allowed to add new keys to the merkle tree, making it create proofs based on a different merkle root than the one signed in the last certificate. Even more worrisome this merkle tree would be then returned modified to its mkpool, making this corruption persist until the pool is refreshed.
By using the "static" method calls.
Alenar
force-pushed
the
djo/1785/last_tx_not_proved
branch
from
404449d to
7b1564b
Compare
It was missing one block range when retrieving data. With the given block ranges in DB (end is exclusive): [ (15..30), (30..45), (45..60) ] Before the fix it would return when asked block number: - `44`: [ (15..30) ] -> WRONG, should also include (30..45) - `45`: [ (15..30) ] -> WRONG, should also include (30..45) - `46`: [ (15..30), (30..45) ] -> WRONG, should also include (45..60)
Substacting `1` to the result to account for the fact that a block range end is exclusive.
* Mithril-aggregator from `0.5.34` to `0.5.35` * Mithril-signer from `0.2.157` to `0.2.158` * Mithril-common from `0.4.25` to `0.4.26` * Mithril-persistence from `0.2.14` to `0.2.15`
Alenar
force-pushed
the
djo/1785/last_tx_not_proved
branch
from
7b1564b to
10e2a20
Compare
12 tasks
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.
Content
This PR includes a fix for #1785, there was several problems:
To solve this we simply subtract one to the computation.
Not directly related to the bug:
This was worrisome since this corrupted merkle tree is returned to a resource pool after use, meaning that a subsequent requests would also yield invalid proofs.
Pre-submit checklist
Comments
I'm unsure this totally fix the problem as when investing the logs I could see that, in the transaction importer, they were a gap of minus one between the block number of the transaction we asked it to import and the really imported transactions.
We should oversee the testing preview network after merge to ensure that its fixed or if there's still more work needed.
Issue(s)
Relates to #1785