Skip to content

refactor cache cleanup logic in UpdateContextForReplacedHeader - #7825

Merged
AdoAdoAdo merged 1 commit into
feat/testnet-fixesfrom
fix-cleanup-postprocess-results-cache
May 19, 2026
Merged

refactor cache cleanup logic in UpdateContextForReplacedHeader#7825
AdoAdoAdo merged 1 commit into
feat/testnet-fixesfrom
fix-cleanup-postprocess-results-cache

Conversation

@AdoAdoAdo

Copy link
Copy Markdown
Contributor

Reasoning behind the pull request

Proposed changes

Testing procedure

Pre-requisites

Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:

  • was the PR targeted to the correct branch?
  • if this is a larger feature that probably needs more than one PR, is there a feat branch created?
  • if this is a feat branch merging, do all satellite projects have a proper tag inside go.mod?

@AdoAdoAdo
AdoAdoAdo marked this pull request as ready for review April 6, 2026 13:42
@AdoAdoAdo
AdoAdoAdo requested a review from Copilot April 6, 2026 14:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors UpdateContextForReplacedHeader to avoid unnecessary (and potentially harmful) cache cleanup when the current execution result matches the execution result that will be set, and updates unit tests accordingly.

Changes:

  • Make cache cleanup conditional on the current execution result being non-nil and having a different header hash than the target execution result.
  • Adjust an existing test to ensure cache cleanup is exercised by using a different current execution result hash.
  • Add a new test verifying cache cleanup is skipped when current and target execution results share the same header hash.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
process/common.go Adds a guard to only clean caches when replacing with a different execution result (by header hash).
process/common_test.go Updates/extends tests to cover both “cleanup called” and “cleanup skipped” scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread process/common.go
Comment on lines +25 to 29
"github.com/multiversx/mx-chain-go/storage"

"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/dataRetriever"
"github.com/multiversx/mx-chain-go/state"

Copilot AI Apr 6, 2026

Copy link

Choose a reason for hiding this comment

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

Import grouping: github.com/multiversx/mx-chain-go/storage is split into its own import block, while other files keep all mx-chain-go/... imports together (e.g. process/interface.go). Consider grouping storage with the other mx-chain-go imports to match the repository convention and avoid goimports/gofmt churn.

Suggested change
"github.com/multiversx/mx-chain-go/storage"
"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/dataRetriever"
"github.com/multiversx/mx-chain-go/state"
"github.com/multiversx/mx-chain-go/common"
"github.com/multiversx/mx-chain-go/dataRetriever"
"github.com/multiversx/mx-chain-go/state"
"github.com/multiversx/mx-chain-go/storage"

Copilot uses AI. Check for mistakes.
@AdoAdoAdo
AdoAdoAdo merged commit 2f9f7c8 into feat/testnet-fixes May 19, 2026
14 of 15 checks passed
@AdoAdoAdo
AdoAdoAdo deleted the fix-cleanup-postprocess-results-cache branch May 19, 2026 07:25
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.

4 participants