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

Initialize precompiles in InitGenesis #57

Closed
wants to merge 2 commits into from

Conversation

evgeniy-scherbina
Copy link

@evgeniy-scherbina evgeniy-scherbina commented May 6, 2024

Moved to #58

PR is deprecated, Initialize precompiles in InitGenesis should follow these guildelines: https://kava-labs.atlassian.net/wiki/spaces/ENG/pages/1469448211/Module+Genesis+Guidelines

PR breaks this requirement: ExportGenesis(InitGenesis(genesisState)) == genesisState, because we call SetNonce & SetCode in InitGenesis which creates evm accounts, then these accounts will be exported in ExportGenesis.

We should instead rely on GenesisAccount{ 0x100…0, 0x01, []State{}) being in the genesis state for each enabled precompile

@evgeniy-scherbina evgeniy-scherbina force-pushed the precompiles-initialization-v2 branch 2 times, most recently from 9894b45 to 98311b4 Compare May 6, 2024 18:43
@evgeniy-scherbina evgeniy-scherbina marked this pull request as ready for review May 6, 2024 19:03
Comment on lines +69 to +73
stateDB.SetNonce(addr, PrecompileNonce)
// Set the code of the precompile's address to a non-zero length byte slice to ensure that the precompile
// can be called from within Solidity contracts. Solidity adds a check before invoking a contract to ensure
// that it does not attempt to invoke a non-existent contract.
stateDB.SetCode(addr, PrecompileCode)
Copy link
Member

Choose a reason for hiding this comment

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

We could use Keeper.SetAccount() for nonce+codehash and Keeper.SetCode directly instead of using StateDB since we don't need the snapshot/rollback/commit features of StateDB.

SetCode also is global mapping of codeHash to code too, so we only need to set the code once (still need to set codehash in account)

Copy link
Author

Choose a reason for hiding this comment

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

This task was split into 2 tasks:

This PR will be closed, sorry for confusion.

Copy link
Member

Choose a reason for hiding this comment

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

ah no worries

Copy link
Author

Choose a reason for hiding this comment

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

and yeah, in #58 I'm directly using evmKeeper instead of StateDB

@@ -68,7 +68,14 @@ func (k *Keeper) NewEVM(
tracer = k.Tracer(ctx, msg, cfg.ChainConfig)
}
vmConfig := k.VMConfig(ctx, msg, cfg, tracer)
return k.evmConstructor(blockCtx, txCtx, stateDB, cfg.ChainConfig, vmConfig)

enabledPrecompiles := k.GetParams(ctx).EnabledPrecompiles
Copy link
Member

Choose a reason for hiding this comment

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

Params is in EVMConfig so we don't need to fetch it again - cfg.Params.EnabledPrecompiles

Copy link
Author

Choose a reason for hiding this comment

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

got it

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

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