Skip to content

Conversation

@ceyonur
Copy link
Collaborator

@ceyonur ceyonur commented Nov 13, 2024

Why this should be merged

Currently shared memory writes version DB to the shared database (avaDB). This conflicts and corrupts the standaloneDB and leads to a duality in data sources: acceptedBlockDB and metaDB were being written to the shared db (avaDB) and chaindb was being written to the standalone DB. This PR removes the unused shared memory components from subnet-evm.

How this works

This pull request includes significant refactoring and cleanup of the plugin/evm package, specifically focusing on the removal of shared memory operations and simplifying database interactions. The most important changes include the removal of the sharedMemoryWriter, modifications to the Accept method, and updates to the test cases to reflect these changes.

  • plugin/evm/block.go: Removed the use of sharedMemoryWriter and related logic from the Accept method and handlePrecompileAccept function. [1] [2] [3]

  • plugin/evm/shared_memory_writer.go: Deleted the entire shared_memory_writer.go file, which included the sharedMemoryWriter implementation.

  • plugin/evm/vm.go: Modified the initializeDBs method to use avaDB directly instead of creating a versioned database.

  • plugin/evm/vm_test.go: Updated test setup to use baseDB directly and added a new test TestStandaloneDB to ensure the standalone database is used correctly. [1] [2]

How this was tested

Added regression UT & locally tested

Need to be documented?

No

Need to update RELEASES.md?

No

@ceyonur ceyonur changed the title add regression test remove shared memory from precompiles and accept Nov 13, 2024
@ceyonur ceyonur marked this pull request as ready for review November 13, 2024 12:33
@ceyonur ceyonur requested review from a team and darioush as code owners November 13, 2024 12:33
Copy link
Contributor

@ARR4N ARR4N left a comment

Choose a reason for hiding this comment

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

I don't know enough about the problem to give a safe approval, but overall LGTM.

@ceyonur ceyonur mentioned this pull request Nov 13, 2024
Co-authored-by: Darioush Jalali <[email protected]>
Signed-off-by: Ceyhun Onur <[email protected]>
darioush
darioush previously approved these changes Nov 13, 2024
@ceyonur ceyonur merged commit ec46ce5 into master Nov 13, 2024
14 checks passed
@ceyonur ceyonur deleted the remove-shared-memory branch November 13, 2024 17:05
github-merge-queue bot pushed a commit that referenced this pull request Nov 20, 2025
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