Move atomic state #709

ceyonur · 2024-12-18T15:02:28Z

Why this should be merged

Moves atomic state (repo, trie backend) to atomic pkg.

How this works

This pull request involves renaming files and updating package references to improve the organization and clarity of the codebase. The changes primarily focus on renaming the plugin/evm package to plugin/atomic and updating relevant imports and type references accordingly.

File Renaming and Package Updates:

Renamed plugin/evm/atomic_backend.go to plugin/atomic/atomic_backend.go and updated the package declaration from evm to atomic.
Renamed plugin/evm/atomic_state.go to plugin/atomic/atomic_state.go and updated the package declaration from evm to atomic.
Renamed plugin/evm/atomic_syncer.go to plugin/atomic/atomic_syncer.go and updated the package declaration from evm to atomic.
Renamed plugin/evm/atomic_syncer_test.go to plugin/atomic/atomic_syncer_test.go and updated the package declaration from evm to atomic.
Renamed plugin/evm/atomic_trie.go to plugin/atomic/atomic_trie.go and updated the package declaration from evm to atomic.

Import and Type Reference Updates:

Updated import paths and type references from evm/atomic to atomic in plugin/atomic/atomic_backend.go.
Updated type references from atomic.Tx to Tx in plugin/atomic/atomic_backend.go.
Updated type references from atomic.Tx to Tx in plugin/atomic/atomic_state.go.
Updated import paths and type references from evm/atomic to atomic in plugin/atomic/atomic_trie.go [1] [2].
Updated import paths and type references from evm/atomic to atomic in plugin/atomic/atomic_trie_iterator_test.go [1] [2].

These changes ensure that the codebase is more organized and maintainable by clearly separating the atomic functionality from the evm package.

How this was tested

Existing tests should cover his

Need to be documented?

No

Need to update RELEASES.md?

No

Signed-off-by: Ceyhun Onur <[email protected]>

Co-authored-by: Quentin McGaw <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

…move-atomic-gossip

darioush

mostly looks good, some minor comments

plugin/evm/syncervm_client.go

plugin/evm/vm_test.go

darioush · 2024-12-30T16:08:09Z

plugin/evm/vm.go

 	return tx, nil
 }
+
+func (vm *VM) PutLastAcceptedID(ID []byte) error {


plugin/evm/syncervm_test.go

darioush · 2024-12-30T16:15:15Z

plugin/evm/syncervm_client.go


-var stateSyncSummaryKey = []byte("stateSyncSummary")
+type BlockAcceptor interface {
+	PutLastAcceptedID([]byte) error


nit: seems this interface should take ids.ID

I did not want to convert it to ID from hash and then to byte for the updateVMMarkers, but yea this sounds weird as it is.

plugin/evm/export_tx_test.go

plugin/evm/atomic/atomic_trie_test.go

darioush · 2024-12-30T16:33:15Z

plugin/evm/vm.go

-	// Prefixes for atomic trie
-	atomicTrieDBPrefix     = []byte("atomicTrieDB")
-	atomicTrieMetaDBPrefix = []byte("atomicTrieMetaDB")


IMO it's somewhat more flexible to let the vm make the prefix and pass the prefixed DB to both the atomic backend and the syncer. We don't need to retain them as fields on the VM.

If you feel strongly that the prefixes should be moved it's fine, but so far the caller has been defining the prefixes.

I think state files/packages should contain prefixes and create prefixdb. Or really the whoever going to use it can create the prefixdb. I can't deny having them in a single place helps to look up, inspect DBs but from the modularity POV it feels like they should be belong to whoever using. In this case we should definetly move them to atomic pkg as we plan to wrap the plugin/vm.go with atomic/vm.go (so no atomic stuff should be in plugin/vm.go)

plugin/evm/vm.go

utils/snow.go

plugin/evm/syncervm_client.go

plugin/evm/atomic/atomic_backend.go

plugin/evm/atomic/atomic_syncer.go

utils/snow.go

plugin/evm/atomic/test_shared_memories.go

Co-authored-by: Darioush Jalali <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

…ove-atomic-state

qdm12 · 2025-01-03T15:16:33Z

RELEASES.md


 ## [v0.14.1](https://github.com/ava-labs/coreth/releases/tag/v0.14.1)
 - Remove API eth_getAssetBalance that was used to query ANT balances (deprecated since v0.10.0)
+- Remove legacy gossip handler and metrics (deprecated since v0.10.0)


Shouldn't this change be in its own PR (maybe before that PR)?

qdm12 · 2025-01-03T15:17:32Z

go.mod

 require (
 	github.com/VictoriaMetrics/fastcache v1.12.1
-	github.com/ava-labs/avalanchego v1.12.1-0.20241209214115-1dc4192013aa
+	github.com/ava-labs/avalanchego v1.12.2-0.20241224181600-fade5be3051d


Why the bump?

qdm12 · 2025-01-03T15:18:57Z

plugin/evm/atomic/atomic_backend.go

+	// Prefixes for atomic trie
+	atomicTrieDBPrefix     = []byte("atomicTrieDB")
+	atomicTrieMetaDBPrefix = []byte("atomicTrieMetaDB")


nit unneeded comment

Suggested change

// Prefixes for atomic trie

atomicTrieDBPrefix = []byte("atomicTrieDB")

atomicTrieMetaDBPrefix = []byte("atomicTrieMetaDB")

atomicTrieDBPrefix = []byte("atomicTrieDB")

atomicTrieMetaDBPrefix = []byte("atomicTrieMetaDB")

qdm12 · 2025-01-03T15:21:04Z

plugin/evm/atomic/atomic_trie.go

+	commitInterval      uint64                     // commit interval, same as commitHeightInterval by default
+	metadataDB          avalanchedatabase.Database // Underlying database containing the atomic trie metadata
+	trieDB              *triedb.Database           // Trie database
+	lastCommittedRoot   common.Hash                // trie root of the most recent commit
+	lastCommittedHeight uint64                     // index height of the most recent commit
+	lastAcceptedRoot    common.Hash                // most recent trie root passed to accept trie or the root of the atomic trie on intialization.


Note for the future/new code (to myself as well 😄) - we should write comments above fields and not to the the right, otherwise gofmt causes unneeded git diffs on other fields as soon as a field name or type gets longer/shorter than the longest/smallest.

qdm12 · 2025-01-03T15:24:54Z

plugin/evm/atomic/tx_heap.go

@@ -1,7 +1,7 @@
-// (c) 2020-2021, Ava Labs, Inc. All rights reserved.
+// (c) 2020-2025, Ava Labs, Inc. All rights reserved.


Should this be updated on other files? 🤔
I'm looking into copyright thingies in ava-labs/libevm#100 because right now it's a bit of headache to keep track of those years 😕

darioush

just nits

plugin/evm/atomic/atomic_trie_iterator_test.go

plugin/evm/atomic/atomic_trie_test.go

plugin/evm/atomic/atomic_tx_repository_test.go

ceyonur and others added 29 commits December 11, 2024 16:57

new atomic pkg

bc26c83

Merge branch 'master' into move-atomic-txs

fe45c37

bump avago

9c3e284

bump versions

ddbbab7

move attomic gossip

0ecd789

lint

019bb04

change newimport clk to time

aa50ce6

move utils

52081a9

remove extra heaps files

d93257b

move database to own pkg

0b10cc4

move atomic trie/state/backend to separaet pkg

8eb3056

Merge branch 'seperate-atomic-pkg-base' into move-atomic-txs

397c535

bump avago

2b13c8e

go mod tidy

2798ad6

Merge branch 'move-atomic-txs' into move-atomic-gossip

6bf2ff2

Signed-off-by: Ceyhun Onur <[email protected]>

update releases md

278e055

use address methods from avago

e5f2c27

bump avago

71babe6

bump e2e avago version

aab47b6

Merge branch 'move-atomic-txs' into move-atomic-gossip

b9b67bc

Update plugin/evm/atomic/gossip_test.go

6f1adc8

Co-authored-by: Quentin McGaw <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

Update plugin/evm/atomic/mempool.go

876716f

Co-authored-by: Quentin McGaw <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

Update plugin/evm/config/config.go

682e4bc

Co-authored-by: Quentin McGaw <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

Update plugin/evm/atomic/tx_heap.go

38ad868

Co-authored-by: Quentin McGaw <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

Update plugin/evm/config/config.go

de328e9

Co-authored-by: Quentin McGaw <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

fix reviews

55c0be9

Merge branch 'move-atomic-gossip' of github.com:ava-labs/coreth into …

3ac1a1a

…move-atomic-gossip

Merge branch 'move-atomic-gossip' into move-atomic-state

9c148ff

fix linter

803e50a

ceyonur marked this pull request as ready for review December 29, 2024 10:02

ceyonur requested review from a team and darioush as code owners December 29, 2024 10:03

ceyonur marked this pull request as draft December 29, 2024 10:03

nits

0b40185

ceyonur marked this pull request as ready for review December 29, 2024 10:32

darioush reviewed Dec 30, 2024

View reviewed changes

qdm12 reviewed Dec 30, 2024

View reviewed changes

Base automatically changed from move-atomic-gossip to master December 31, 2024 08:39

ceyonur changed the base branch from master to seperate-atomic-pkg-base January 3, 2025 10:16

ceyonur and others added 4 commits January 3, 2025 13:30

Update plugin/evm/vm_test.go

2f1dbc5

Co-authored-by: Darioush Jalali <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

Update plugin/evm/syncervm_test.go

936bb59

Co-authored-by: Darioush Jalali <[email protected]> Signed-off-by: Ceyhun Onur <[email protected]>

review fix

4019a34

Merge branch 'move-atomic-state' of github.com:ava-labs/coreth into m…

266a55f

…ove-atomic-state

qdm12 approved these changes Jan 3, 2025

View reviewed changes

darioush approved these changes Jan 3, 2025

View reviewed changes

ceyonur self-assigned this Jan 6, 2025

ceyonur added the atomic refactor label Jan 6, 2025

Merge branch 'seperate-atomic-pkg-base' into move-atomic-state

1958ee8

ceyonur merged commit b12462a into seperate-atomic-pkg-base Jan 7, 2025
6 checks passed

ceyonur deleted the move-atomic-state branch January 7, 2025 13:45

		@@ -1,7 +1,7 @@
		// (c) 2020-2021, Ava Labs, Inc. All rights reserved.
		// (c) 2020-2025, Ava Labs, Inc. All rights reserved.

Uh oh!

Move atomic state #709

Move atomic state #709

Uh oh!

Conversation

ceyonur commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this should be merged

How this works

File Renaming and Package Updates:

Import and Type Reference Updates:

How this was tested

Need to be documented?

Need to update RELEASES.md?

Uh oh!

darioush left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darioush left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ceyonur commented Dec 18, 2024 •

edited

Loading