Skip to content

Conversation

@svlachakis
Copy link
Contributor

@svlachakis svlachakis commented Aug 28, 2025

Changes needed for NethermindEth/nethermind-arbitrum#167

In Arbitrum we have Precompiled Addresses that need to be handled by isPrecompile function in AddressExtensions.cs
Since the method is static and we can't put Arbitrum related logic there we need to make this method extensible.

With those changes, in Arbitrum we will implement ArbitrumPrecompileChecker : IPrecompileChecker and add all Arbitrum related addresses there.

We will also use CompositePrecompileChecker in the Arbitrum plugin to combine Ethereum and Arbitrum precompiles.

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

@svlachakis svlachakis changed the title IsPrecompiles Extensibility AddressExtensions.IsPrecompiles Extensibility Aug 28, 2025
@svlachakis svlachakis marked this pull request as ready for review August 28, 2025 14:05
@svlachakis svlachakis changed the title AddressExtensions.IsPrecompiles Extensibility AddressExtensions.IsPrecompiles - Extensibility Aug 28, 2025
@LukaszRozmej
Copy link
Member

LukaszRozmej commented Aug 29, 2025

Not sure if I like it, what do you think about alternatives and store that info in IReleaseSpec, like:

  1. IReleaseSpec.IsPrecompile(Address)
  2. HashSet<AddressAsKey> IReleaseSpec.Precompiles

?

Similarly how VirtualMachine.PrepareOpcodes already uses IReleaseSpec to do it.

@benaadams

@wurdum
Copy link
Contributor

wurdum commented Aug 29, 2025

I strongly support Lukasz idea. It feels right.

Another thing is that with the current scope of changes, supporting feature/arbitrum-setup is going to be challenging (even mid-term). I'd either look for an alternative solution or set master as PR target.

@svlachakis
Copy link
Contributor Author

svlachakis commented Aug 30, 2025

@damian-orzechowski @LukaszRozmej @wurdum I've implemented @LukaszRozmej 's idea since these changes are a lot and possibly risky - I definitely agree and would appreciate your thoughts!

#9216
Update : #9222 with master as target branch

@svlachakis svlachakis marked this pull request as draft August 30, 2025 20:52
@svlachakis svlachakis closed this Sep 1, 2025
@svlachakis svlachakis deleted the precompiles-extension branch September 1, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants