search

aergoio / aergo

aergo blockchain kernel
http://docs.aergo.io
MIT License
214 stars 44 forks source link

refactor state #331

Closed rabbitprincess closed 9 months ago

rabbitprincess commented 9 months ago

What is changed?

add id field in ContractState ( to identify AccountState in vm ) move ContractState to statedb ( it related with evm_state_wip ) change balance modify logic in vm.go to use AccountState (luaSendAmount, luaCallContract, luaDeployContract) replace add/sub balance logic to state.SendBalance() fix some tests in vm and governance

Is there risk of fork?

yes. it need to sync test.

kroggen commented 9 months ago

OK, a sync test can be run to check its backwards compatibility

kslee8282 commented 9 months ago

sync start [mainnet] Dec 7 05:02:14.485 INF ../go/aergo/cmd/aergosvr/aergosvr.go:98 > AERGO SVR STARTED branch=feature/refactor-state module=asvr revision=f7b6e39f

kslee8282 commented 9 months ago

sync success

rabbitprincess commented 9 months ago

@kroggen can you approve this pr?

kroggen commented 9 months ago

I will check for conflicts, tomorrow

kroggen commented 9 months ago

@gokch

Why AccountState is on state and ContractState is on statedb?

I am also curious why many functions were moved from state to statedb

kroggen commented 9 months ago

The separation of vmAutoload, vmLoadCode and vmLoadCall was completely unnecessary. They are related and were together at the end of the file. I only now saw this

rabbitprincess commented 9 months ago

@gokch

Why AccountState is on state and ContractState is on statedb?

I am also curious why many functions were moved from state to statedb

contractState is only modified by lua contract, but accountState is modified by lua contract and external chain. I tried to collect only changed state by lua into statedb. ( but, statedb is not clear package name. )

and, Although it is not confirmed, this also considering evm support. evm and lua have their own state db, but they share same accountState.

rabbitprincess commented 9 months ago

The separation of vmAutoload, vmLoadCode and vmLoadCall was completely unnecessary. They are related and were together at the end of the file. I only now saw this

yes, and it is also good to separate vmContext and executor. ( make new file vm_executor.go ) Current pr`s purpose is for refactor state, so it seems better to change it in another pr.