Skip to content
This repository has been archived by the owner on Mar 5, 2024. It is now read-only.

use old ledger when determining target #1379

Merged
merged 1 commit into from Jun 2, 2022

Conversation

andymck
Copy link
Contributor

@andymck andymck commented Jun 2, 2022

During receipt v2 validations, the incorrect ledger was being used when determining the target. This could have resulted in valid receipts being rejected and is the probable cause for most of the "receipt not in order" errors.

thanks to community member BigEnigma for spotting this

NOTE: i have gated this behind the h3dex targeting chain var. The fix will come into use once h3dex is enabled

@andymck andymck force-pushed the andymck/fix-receiptv2-pathing branch from a180192 to e25b58d Compare June 2, 2022 11:23
@BigEnigma
Copy link

Hey Mack

Looked at the change, not wanting to push my luck but one thing i'm not sure about shouldn't the targeting be based on the module defined in the vars/config at the time the transaction was run so its uses the right method?

BigE

@BigEnigma
Copy link

:) me again, sorry i noticed you applied this such that it would only kick in for target V6 and above, was this because prior to that we cannot reliable replay anything should anyone do so? this ones a curiosity question in just how to go about managing versioned logic against version transactions and data models :)

@madninja
Copy link
Member

madninja commented Jun 2, 2022

:) me again, sorry i noticed you applied this such that it would only kick in for target V6 and above, was this because prior to that we cannot reliable replay anything should anyone do so? this ones a curiosity question in just how to go about managing versioned logic against version transactions and data models :)

That's correct.. If we don't do this as part of v6 then replaying the chain would cause it to use newer code on older blocks which would be consensus breaking

@madninja madninja merged commit 88ce54f into master Jun 2, 2022
@madninja madninja deleted the andymck/fix-receiptv2-pathing branch June 2, 2022 22:19
@BigEnigma
Copy link

Hi

Did my first question about the variable used to get the targeting version make sense, is that bit ok going forward if we get to v7 but something gets an old txn from before a chain var switch should it not use v6 targeting instead of the current configured v7?

BigE

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.

None yet

4 participants