Skip to content
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

Fix receipt recovery & migration #5989

Merged
merged 7 commits into from
Aug 9, 2023
Merged

Conversation

emlautarom1
Copy link
Contributor

@emlautarom1 emlautarom1 commented Aug 7, 2023

Fixes #5823

Changes

  • Use AncientReceiptsBarrier as lower bound when fixing receipts.
  • Properly calculate receipt.ContractAddress by recovering sender.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • [] No

Notes on testing

Added unit test and also tested against user provided scenario on a running client.

Documentation

Requires documentation update

  • Yes
  • No

Requires explanation in Release Notes

  • Yes
  • No

Remarks

It would be nice if we could create a unit/integration test for the associated ticket.

MissingReceiptsFixVisitor visitor = new MissingReceiptsFixVisitor(
syncConfig.PivotNumberParsed,
MissingReceiptsFixVisitor visitor = new(
syncConfig.AncientReceiptsBarrierCalc,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

syncConfig.PivotNumberParsed delegates to syncConfig.PivotNumber, which is not a read-only field. The pivot number is modified here:

_syncConfig.PivotNumber = updatedPivotBlockNumber.ToString();

then, by the time that this code gets executed, the pivot number is not what the user originally configured.


Either way, we should use the AncientReceiptsBarrier value for this operation.

@emlautarom1 emlautarom1 changed the title Fix/receipt fix migration Fix receipt migration Aug 7, 2023
@emlautarom1 emlautarom1 self-assigned this Aug 7, 2023
@emlautarom1 emlautarom1 changed the title Fix receipt migration Fix receipt recovery & migration Aug 7, 2023
@emlautarom1 emlautarom1 merged commit 38974ec into master Aug 9, 2023
61 checks passed
@emlautarom1 emlautarom1 deleted the fix/receipt_fix_migration branch August 9, 2023 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gnosis: eth_getTransactionReceipt missing contractAddress
4 participants