Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge master to atomic base branch #744

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

ceyonur
Copy link
Collaborator

@ceyonur ceyonur commented Jan 10, 2025

Merges master (at b6b4dfb) to atomic base branch

@ceyonur ceyonur requested review from darioush and a team as code owners January 10, 2025 12:24
@ceyonur ceyonur marked this pull request as draft January 10, 2025 13:23
@ceyonur ceyonur changed the title merge master merge master to atomic base branch Jan 13, 2025
@ceyonur ceyonur marked this pull request as ready for review January 13, 2025 17:41
@ceyonur ceyonur self-assigned this Jan 13, 2025
Copy link
Collaborator

@darioush darioush left a comment

Choose a reason for hiding this comment

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

optional: we can also move the sync, state, txpool packages to "internal" if desired

@@ -68,7 +75,7 @@ type AtomicBackend interface {
}

// atomicBackend implements the AtomicBackend interface using
// the AtomicTrie, AtomicTxRepository, and the VM's shared memory.
// the interfaces.AtomicTrie, AtomicTxRepository, and the VM's shared memory.
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this comment still correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@@ -140,7 +144,7 @@ func newAtomicTrie(
}
// initialize to EmptyRootHash if there is no committed root.
if root == (common.Hash{}) {
root = types.EmptyRootHash
root = EmptyAtomicRootHash
Copy link
Collaborator

Choose a reason for hiding this comment

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

optional: we can import this from go-ethereum's upstream type if we don't want to duplicate this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I reverted it since it should be good after libevm transition

_ AtomicTrie = &atomicTrie{}
lastCommittedKey = []byte("atomicTrieLastCommittedBlock")
appliedSharedMemoryCursorKey = []byte("atomicTrieLastAppliedToSharedMemory")
AtomicKeyLength = wrappers.LongLen + common.HashLength
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: should we rename this to AtomicTrieKeyLength to be more descriptive?

@ceyonur
Copy link
Collaborator Author

ceyonur commented Jan 14, 2025

optional: we can also move the sync, state, txpool packages to "internal" if desired

We cannot move it in this PR as we use them in plugin/evm. We might be able to move them once we isolate the atomic pkg. (golang only allows internal pkgs to be used from their root)

@ceyonur ceyonur merged commit 11bc6e8 into seperate-atomic-pkg-base Jan 14, 2025
6 checks passed
@ceyonur ceyonur deleted the merge-master-atomic-base branch January 14, 2025 08:57
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.

2 participants