Adding move precompiled support #3385

nagarev · 2025-11-06T03:06:08Z

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
Tests for the changes have been added (for bug fixes / features)
Requires Activation Code (Hard Fork)

Other information:

github-actions · 2025-11-06T03:06:26Z

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

fmacleal

Very good job! I liked the refactor, the tests and the way you implemented the logic.

I have a few comments, but nothing big. Well done. 🙂

fmacleal · 2025-11-16T08:14:50Z

rskj-core/src/main/java/co/rsk/rpc/modules/eth/EthModule.java

        Block block = result.getBlock();

-        MutableRepository mutableRepository = prepareRepository(result, block, shouldPerformStateOverride, accountOverrideList);
+        OverrideablePrecompiledContracts overrideablePrecompiledContracts = new OverrideablePrecompiledContracts(precompiledContracts);


suggestion:

Since this is always the same, because the list of PrecompiledContracts doesn't change, maybe we could initialize this object just once in the constructor when the EthModule is initialized. WDYT?

Really good catch!

rskj-core/src/main/java/org/ethereum/vm/OverrideablePrecompiledContracts.java

rskj-core/src/test/java/co/rsk/rpc/modules/eth/DefaultStateOverrideApplierTest.java

rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleDSLTest.java

rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java

Copilot

Pull request overview

This PR adds support for moving precompiled contracts to different addresses using state override functionality. The key feature allows callers to temporarily relocate precompiled contracts (like the Identity/datacopy precompile) to alternative addresses during eth_call operations for testing and simulation purposes.

Key Changes:

Introduced OverrideablePrecompiledContracts wrapper to manage temporary precompile relocations
Extended AccountOverride to support movePrecompileToAddress parameter
Updated state override application logic to handle precompile movement
Added restrictions preventing movement of critical precompiles (Bridge, REMASC)

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`OverrideablePrecompiledContracts.java`	New class that wraps PrecompiledContracts and manages address overrides for precompiled contracts
`AccountOverride.java`	Added movePrecompileToAddress field and validation logic to prevent moving critical contracts
`DefaultStateOverrideApplier.java`	Updated to handle precompile movement and track state override operations
`EthModule.java`	Modified to use OverrideablePrecompiledContracts and pass through override context
`ReversibleTransactionExecutor.java`	Extended to accept and propagate precompiled contracts parameter
`TransactionExecutorFactory.java`	Added overloads to support optional PrecompiledContracts parameter
Test files	Updated test instantiations to use OverrideablePrecompiledContracts wrapper
DSL test file	New test scenario validating precompile movement functionality

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

rskj-core/src/test/resources/dsl/eth_module/test_state_override_move_precompiled.txt

Copilot · 2025-11-26T13:40:46Z

rskj-core/src/main/java/co/rsk/rpc/modules/eth/AccountOverride.java

+    private boolean isMovableContract(RskAddress contractAddress) {
+        byte[] addressBytes = contractAddress.getBytes();
+        return !Arrays.equals(BRIDGE_ADDR.getBytes(), addressBytes) && !Arrays.equals(REMASC_ADDR.getBytes(), addressBytes);
+    }


The method calls getBytes() on BRIDGE_ADDR and REMASC_ADDR constants for every invocation. Consider caching these byte arrays as static final fields to avoid repeated array allocations.

rskj-core/src/main/java/org/ethereum/vm/OverrideablePrecompiledContracts.java

rskj-core/src/main/java/co/rsk/rpc/modules/eth/DefaultStateOverrideApplier.java

Copilot · 2025-11-26T13:40:47Z

rskj-core/src/test/java/co/rsk/rpc/modules/eth/EthModuleTest.java


    @Test
-    void testCall_whenCalledWithAccountOverrideOverPrecompileContractAddress_throwsExceptionAsExpected() {
+    void testCall_whenCalledWithAccountOverrideOverPrecompileContractAddressAndStateIsOverridden_throwsExceptionAsExpected() {


[nitpick] The test method name is very long (113 characters). Consider a more concise name like testCall_accountOverrideOnPrecompileWithState_throwsException to improve readability.

Suggested change

void testCall_whenCalledWithAccountOverrideOverPrecompileContractAddressAndStateIsOverridden_throwsExceptionAsExpected() {

void testCall_accountOverrideOnPrecompileWithState_throwsException() {

fmacleal

Good job!

Although, I still have some comments on regards of where you added the verification for the movable precompiled contracts.

I would it to the StateOverride classes, like DefaultStateOverrideApplier or OverrideablePrecompiledContract. They should hold the business logic. I see the AccountOverride more like a DTO object that shouldn't hold these kinds of logic.

rskj-core/src/main/java/co/rsk/rpc/modules/eth/AccountOverride.java

rskj-core/src/main/java/org/ethereum/vm/OverrideablePrecompiledContracts.java

rskj-core/src/main/java/co/rsk/rpc/modules/eth/DefaultStateOverrideApplier.java

fmacleal

Thanks for having addressed the last issues Naza,

Good job!

sonarqubecloud · 2025-11-28T16:26:43Z

Quality Gate passed

Issues
36 New issues
0 Accepted issues

Measures
0 Security Hotspots
98.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

fmacleal

Good job, thanks for the latest changes!

nagarev force-pushed the nagarev/state-override-move-precompiled branch 2 times, most recently from b503b83 to 161ecdb Compare November 14, 2025 03:38

nagarev marked this pull request as ready for review November 14, 2025 13:36

fmacleal reviewed Nov 16, 2025

View reviewed changes

fmacleal force-pushed the nagarev/state-override-move-precompiled branch from 79d18ef to 57eaac0 Compare November 18, 2025 18:06

nagarev force-pushed the nagarev/state-override-move-precompiled branch 2 times, most recently from eda076b to 0f47d4c Compare November 24, 2025 19:44

fmacleal requested a review from Copilot November 26, 2025 13:39

Copilot AI reviewed Nov 26, 2025

View reviewed changes

fmacleal reviewed Nov 26, 2025

View reviewed changes

github-advanced-security bot found potential problems Nov 27, 2025

View reviewed changes

rskj-core/src/main/java/co/rsk/rpc/modules/eth/DefaultStateOverrideApplier.java Fixed Show fixed Hide fixed

nagarev added 6 commits November 28, 2025 10:51

Adding move precompiled support

ccf0a93

Refactoring state override applier

5b7afc5

Updating tests

5a18404

Refactoring methods

0d88565

Adding tests

d12e70c

Adding DSL test

3e12bb4

nagarev force-pushed the nagarev/state-override-move-precompiled branch from 3a3f0f6 to 7dab9b6 Compare November 28, 2025 13:52

fmacleal previously approved these changes Nov 28, 2025

View reviewed changes

nagarev dismissed fmacleal’s stale review via 7d10218 November 28, 2025 16:07

Refactoring validations

fff34f0

nagarev force-pushed the nagarev/state-override-move-precompiled branch from 7d10218 to fff34f0 Compare November 28, 2025 16:08

fmacleal approved these changes Nov 30, 2025

View reviewed changes

	void testCall_whenCalledWithAccountOverrideOverPrecompileContractAddressAndStateIsOverridden_throwsExceptionAsExpected() {
	void testCall_accountOverrideOnPrecompileWithState_throwsException() {

Adding move precompiled support #3385

Are you sure you want to change the base?

Adding move precompiled support #3385

Uh oh!

Conversation

nagarev commented Nov 6, 2025

Description

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Uh oh!

github-actions bot commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency Review

Scanned Files

Uh oh!

fmacleal left a comment

Choose a reason for hiding this comment

Uh oh!

fmacleal Nov 16, 2025

Choose a reason for hiding this comment

Uh oh!

nagarev Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Nov 26, 2025

Choose a reason for hiding this comment

Uh oh!

fmacleal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fmacleal left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Nov 28, 2025

Quality Gate passed

Uh oh!

fmacleal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Nov 6, 2025 •

edited

Loading