Skip to content

feat: Add integration of Arbitrum precompiles with EVM (with ArbInfo example)#24

Merged
hudem1 merged 52 commits into
mainfrom
hudem1/feat/precompile/arb-info
Jun 18, 2025
Merged

feat: Add integration of Arbitrum precompiles with EVM (with ArbInfo example)#24
hudem1 merged 52 commits into
mainfrom
hudem1/feat/precompile/arb-info

Conversation

@hudem1

@hudem1 hudem1 commented Jun 3, 2025

Copy link
Copy Markdown
Collaborator

Please first review #21 as this one is rebased on it.

This PR depends on some changes to the Nethermind client to make the VirtualMachine overridable.
Here is the related PR on the Nethermind repo.

Comment thread src/Nethermind.Arbitrum/Precompiles/MethodIdHelper.cs Outdated
Comment thread src/Nethermind.Arbitrum/Precompiles/MethodIdHelper.cs
Comment thread src/Nethermind.Arbitrum/Precompiles/Parser/ArbInfoParser.cs
Comment thread src/Nethermind.Arbitrum/TransactionProcessing/ArbitrumTransactionProcessor.cs Outdated
Comment thread src/Nethermind.Arbitrum/TransactionProcessing/ArbitrumTransactionProcessor.cs Outdated
Comment thread src/Nethermind.Arbitrum/Precompiles/Parser/ArbInfoParser.cs
Comment thread src/Nethermind.Arbitrum/Precompiles/Parser/ArbInfoParser.cs
Comment thread src/Nethermind.Arbitrum.Test/Precompiles/Parser/ArbInfoParser.cs
Comment thread src/Nethermind.Arbitrum.Test/Precompiles/Parser/ArbInfoParser.cs
Comment thread src/Nethermind.Arbitrum/ArbitrumPlugin.cs Outdated
Comment thread src/Nethermind.Arbitrum/ArbitrumNethermindApi.cs
Comment thread src/Nethermind.Arbitrum/Arbos/ArbosState.cs Outdated
Comment thread src/Nethermind.Arbitrum/Precompiles/Context.cs Outdated
Comment thread src/Nethermind.Arbitrum/Precompiles/Context.cs Outdated
Comment thread src/Nethermind.Arbitrum/Precompiles/Context.cs Outdated
Comment thread src/Nethermind.Arbitrum/TransactionProcessing/ArbitrumTransactionProcessor.cs Outdated
@hudem1 hudem1 requested a review from AnkushinDaniil June 17, 2025 11:31

@AnkushinDaniil AnkushinDaniil left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Great job!
I don't have much context on this, but I have a few questions:

  1. Should gas be burned before expensive operations?
  2. Should introduced code be thread-safe?

Feel free to merge this PR and address these questions later.

Comment thread src/Nethermind.Arbitrum/Precompiles/Parser/ArbInfoParser.cs
Comment thread src/Nethermind.Arbitrum.Test/Precompiles/Parser/ArbInfoParser.cs
@hudem1

hudem1 commented Jun 17, 2025

Copy link
Copy Markdown
Collaborator Author

Great job! I don't have much context on this, but I have a few questions:

1. Should gas be burned before expensive operations?

2. Should introduced code be thread-safe?

Feel free to merge this PR and address these questions later.

Great job! I don't have much context on this, but I have a few questions:

1. Should gas be burned before expensive operations?

2. Should introduced code be thread-safe?

Feel free to merge this PR and address these questions later.

  1. Yes gas is burned inside the precompile methods. ArbInfoParser.cs actually decodes the data into the expected types and pass the decoded data as parameters when calling the actual precompile implementation ArbInfo.cs. In that precompile, gas is burned in each precompile method.
  2. That precompile in particular is just read-only, just getting data. But maybe for future precompiles that update the state, we should use thread safe code ? 🤔 @damian-orzechowski

@damian-orzechowski

damian-orzechowski commented Jun 18, 2025

Copy link
Copy Markdown
Collaborator
  1. That precompile in particular is just read-only, just getting data. But maybe for future precompiles that update the state, we should use thread safe code ? 🤔 @damian-orzechowski

Currently I think there is not need for that as there is only a single transaction / EVM processing thread. Unless these precompiles are to be executed elsewhere with possible multithreading, then I don't think it's a requirement (for now).

@hudem1 hudem1 merged commit 022576e into main Jun 18, 2025
1 check passed
@hudem1 hudem1 deleted the hudem1/feat/precompile/arb-info branch June 18, 2025 10:31
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