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

Precompile pre post handling #524

Merged
merged 28 commits into from
Mar 16, 2023
Merged

Conversation

aaronbuchwald
Copy link
Collaborator

Why this should be merged

This PR adds precompile predicate enforcement and onAccept handling that will be used by the warp and shared memory precompiles.

How this works

This PR adds optional Accepter and Predicater interfaces, which are to be optionally implemented by precompile contracts (contract not config or module).

These optional interfaces are NOT intended to be used for custom precompiles outside of this repo and the comments in this PR reflect that. Making his an optional interface removes the need to include it in the precompile gen template and makes it so that a user of the precompilegen template does not even need to see or think about the Accepter or Predicater interface at all.

The predicate is enforced on the relevant access list when calling VerifyWithContext or BuildWithContext.

The Accept handling is performed during block acceptance within plugin/evm/block.go. This is a change from a previous implementation to simplify passing in the backend, so that it does not require changing as much code.

Additionally, the shared memory precompile will need to perform shared memory operations to be committed atomically alongside updating the block that is marked as last accepted.

I spent some time trying to make this logic work within core/blockchain.go, but found that it will be easier to maintain this within plugin/evm/block.go.

How this was tested

This was tested via CI and should be a no-op change since no precompiles currently use the added optional interfaces.

Now that we have the module registration logic, we can implement a test case for this that includes its own implementation of a precompile that uses a predicate and on accept to test this.

How is this documented

This is not intended for end users to touch, so there is not currently an accompanying documentation PR. We should document this in the future, but deferring that for now.

@aaronbuchwald aaronbuchwald added this to the Subnet-EVM@v0.4.10 milestone Feb 17, 2023
@aaronbuchwald aaronbuchwald self-assigned this Feb 17, 2023
@aaronbuchwald
Copy link
Collaborator Author

TODO: clearly document new invariants and update precompilegen tutorial with any necessary changes.

Now that this uses optional interfaces for accepter and predicater, this should not require any changes on top of what's already in the precompile refactor.

@aaronbuchwald aaronbuchwald added the enhancement New feature or request label Feb 18, 2023
miner/worker.go Outdated Show resolved Hide resolved
miner/worker.go Outdated Show resolved Hide resolved
plugin/evm/block.go Outdated Show resolved Hide resolved
plugin/evm/vm.go Outdated Show resolved Hide resolved
Base automatically changed from precompile-improvements-main to master February 22, 2023 20:27
@aaronbuchwald aaronbuchwald changed the base branch from master to HelloWorldOfficialTutorial February 22, 2023 20:30
@aaronbuchwald aaronbuchwald changed the base branch from HelloWorldOfficialTutorial to master February 22, 2023 20:30
core/state/statedb.go Outdated Show resolved Hide resolved
utils/bytes.go Outdated Show resolved Hide resolved
@aaronbuchwald aaronbuchwald added the DO NOT MERGE This PR is not meant to be merged in its current state label Feb 22, 2023
@aaronbuchwald
Copy link
Collaborator Author

Marking as DO NOT MERGE until after the v0.4.10 release with the precompile refactor.

@aaronbuchwald aaronbuchwald marked this pull request as ready for review February 22, 2023 21:02
core/state/statedb.go Outdated Show resolved Hide resolved
@aaronbuchwald aaronbuchwald mentioned this pull request Mar 2, 2023
6 tasks
core/predicate_check_test.go Outdated Show resolved Hide resolved
core/predicate_check_test.go Outdated Show resolved Hide resolved
core/state/statedb.go Outdated Show resolved Hide resolved
plugin/evm/block.go Outdated Show resolved Hide resolved
plugin/evm/block.go Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
precompile/precompileconfig/config.go Outdated Show resolved Hide resolved
precompile/precompileconfig/config.go Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
Comment on lines +590 to +598
// PrecompilePredicates maps addresses to stateful precompile predicate functions
// that are enabled for this rule set.
PredicatePrecompiles map[common.Address]precompileconfig.PrecompilePredicater
// ProposerPredicates maps addresses to stateful precompile predicate functions
// that are enabled for this rule set and require access to the ProposerVM wrapper.
ProposerPredicates map[common.Address]precompileconfig.ProposerPredicater
// AccepterPrecompiles map addresses to stateful precompile accepter functions
// that are enabled for this rule set.
AccepterPrecompiles map[common.Address]precompileconfig.Accepter
Copy link
Collaborator

Choose a reason for hiding this comment

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

noice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤝

params/config.go Show resolved Hide resolved
plugin/evm/block.go Show resolved Hide resolved
Comment on lines +208 to +213
return b.verify(&precompileconfig.ProposerPredicateContext{
PrecompilePredicateContext: precompileconfig.PrecompilePredicateContext{
SnowCtx: b.vm.ctx,
},
ProposerVMBlockCtx: proposerVMBlockCtx,
}, true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we split new predicate context to a variable and then use b.verify(predicateContext, true) for readability? we can also create a function for NewProposerPredicateContext which would take ctx and proporservmblockctx as inputs (not really sure how custom we would want this to be though)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Going to leave this as is, since I don't think this would really improve readability.

precompile/precompileconfig/config.go Outdated Show resolved Hide resolved
// WARNING: If you are implementing a custom precompile, beware that subnet-evm
// will not maintain backwards compatibility of this interface and your code should not
// rely on this. Designed for use only by precompiles that ship with subnet-evm.
type ProposerPredicater interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit confused about the difference between ProposerPredicate vs PrecompilePredicate. will precompiles implement both of these interfaces?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adding a comment for this. Precompiles should implement ProposerPredicate iff they depend on the ProposerVM Block Context ie. Warp.

@aaronbuchwald aaronbuchwald merged commit 8f8ae38 into master Mar 16, 2023
@aaronbuchwald aaronbuchwald deleted the precompile-pre-post-handling branch March 16, 2023 15:34
atvanguard pushed a commit to hubble-exchange/hubblenet that referenced this pull request Mar 27, 2023
* - Adding missing import "encoding/json" (used in Stringer-method) (ava-labs#465)

- re-ordering of imports

* fix allow list comments (ava-labs#469)

* fix allow list comments

* cleaner sentences

* use preallocated big nums

* infer allow list role type

* add nil checks (ava-labs#473)

* add nil checks

* add unit test

* use non nil config

* use non-nil configs

---------

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Warp backend interface and implementation (ava-labs#452)

* base warp backend

* add signature caching

* add docs

* error handling

* pr fixes

* hash unsigned message for key

* quick pr fixes and merge

* save signature instead of whole msg

* use avaGO cache

* rename warpBackend and docs

* fix nits

* Update plugin/evm/warp_backend.go

* Update plugin/evm/warp_backend.go

* fix pr nits

* pr fixes and testing

* type check for caching

* fix imports

* use memdb and remove extra test

* remove unused

* fix imports

* saving message in db and pr fixes

* update copyright

* update backend variable naming

* add comment about saving db vs cache

* Add documentation section to PR template (ava-labs#484)

* remove current rules (ava-labs#481)

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* add documentation guidelines (ava-labs#486)

* add documentation guidelines

* fix

* parseInt from ENV var (ava-labs#491)

* Add generate precompile script to fix CGO flags issue (ava-labs#489)

* try to fix lint job (ava-labs#499)

* build with avago v1.9.8 (ava-labs#498)

* build with avago v1.9.8

* add indirect deps

* build fix

* another build fix

* try to fix lint job

* Signature Request Handler (ava-labs#459)

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Update codeowners (ava-labs#492)

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Get signature endpoint: alternative PR with different packaging (ava-labs#507)

* base warp backend

* add signature caching

* add docs

* error handling

* pr fixes

* basic signature request

* hash unsigned message for key

* implement new Request and RequestHandler interfaces

* signature handler impl without constructing one

* fix import

* quick pr fixes and merge

* quick pr fixes and merge

* save signature instead of whole msg

* use avaGO cache

* rename warpBackend and docs

* fix nits

* Update plugin/evm/warp_backend.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Update plugin/evm/warp_backend.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* fix pr nits

* pr fixes and testing

* type check for caching

* handlers and request before tests

* fix imports

* signature handler with stats and test

* use memdb and remove extra test

* remove unused

* fix imports

* fix imports

* nit

* update license year

* use require noError

* saving message in db and pr fixes

* create noop signature handler and refactor code handler

* get signature endpoint

* add api arg to evm client

* Update sync/handlers/handler.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* update backend return value

* refactor handlers to network handler

* change constructor of handler stats

* pr cleanups

* warp api

* initialize warp backend

* build fix

* wip

* warp api follows eth api pattern

* cleanup and comments

* clean up response

* fix warp client return type

* nits for get-signature-endpoint (ava-labs#502)

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: Ceyhun Onur <ceyhun.onur@avalabs.org>
Co-authored-by: cam-schultz <78878559+cam-schultz@users.noreply.github.com>

* resolve merge conflict

* warp: Group packages for symmetry w/ sync

* more reshuffle

* more shuffle

* pr comments

* fix

* update to []byte

* update svc return type

* rename arg

* fix type

* add stats pkg

---------

Co-authored-by: Matthew Lam <matthew.lam@avalabs.org>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: cam-schultz <camschultz32@gmail.com>
Co-authored-by: Ceyhun Onur <ceyhun.onur@avalabs.org>
Co-authored-by: cam-schultz <78878559+cam-schultz@users.noreply.github.com>

* Update github actions to ignore rcs (ava-labs#521)

* Remove unnecessary gasprice updater logic and tests (ava-labs#514)

* Remove unnecessary gasprice updater logic and tests

* Remove comment referencing gas price updater

* Revert default price limit change

* Move set min fee back to vm.go

* Update to retain previous tx pool gas price setting behavior

* Bump avalanchego to v1.9.9-rc.4 (ava-labs#526)

* Bump avalanchego to v1.9.9-rc.4

* Fix imports

* Bump compatibility.json for latest avalanchego release

* Bump AvalancheGo dep to v1.9.9 (ava-labs#530)

* Update AvalancheGo compatibility (ava-labs#531)

* Start v0.4.10 release cycle (ava-labs#533)

* Stateful Precompile Improvements (ava-labs#389)

* move inline string ABIs to separate files and embed them (ava-labs#383)

* move inline string ABIs to separate files and embed them

* fix tests

* fix tests

* unexport function

* Update accounts/abi/bind/bind.go

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* fix func name

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* replace getByKey with getByAddress (ava-labs#395)

* rework on panics in precompiles (ava-labs#418)

* rework on panics in precompiles

* Update precompile/allow_list.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Update precompile/fee_config_manager.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Update precompile/fee_config_manager.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* fix reviews

* wrap errors in ConfigurePrecompiles

* cleaner errors

* Update utils.go

* Update miner/worker.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Update core/state_processor.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Precompile Specific Packages (ava-labs#420)

* rework on panics in precompiles

* Update precompile/allow_list.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Update precompile/fee_config_manager.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Update precompile/fee_config_manager.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* fix reviews

* wrap errors in ConfigurePrecompiles

* cleaner errors

* move reward manager precompile to package (WIP)

* rename files

* fix abi path

* move typecheck

* move precompiles to their own packages

* refactor precompile template

* remove test file

* upate comments

* rm test files

* new allowlist package

* Update precompile/utils.go

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Update precompile/nativeminter/contract_native_minter.go

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Update precompile/nativeminter/contract_native_minter.go

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Update precompile/utils.go

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Update precompile/nativeminter/contract_native_minter.go

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* fix nits

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* rename fee manager config struct (ava-labs#427)

* rename struct

* rename fee config managers to fee managers

* fix comments

* Generalized upgrades rb (ava-labs#434)

* introduce precompiles as registrable modules

* add precompile specific contract tests

* remove print debug

* add unmarshal tests

* remove unnecessary func

* fix initial disabled value

* register all modules in core/evm/contract_stateful

* more refactor & test fix

* sync template

* fix more tests

* rename file

* add comment

* rename

* fix linter

* use require error contains

* remove whitespace

* trim mock interface

* sort steps

* reviews

* Update precompile/stateful_precompile_module.go

* Update params/precompile_config.go

* Update params/precompile_config.go

* fix reviews

* add new module to configs and group module functions

* generalized-upgrades-rb review (ava-labs#474)

* keep genesis disabled fix

* nits

* nits

* nit

* review fixes

* Update precompile/allowlist/allowlist.go

* use address in map

* fix linter for embedded keys

* update err messages

* more err update

* remove unnecessary function (ava-labs#478)

* Start work on breaking cyclic dependency (ava-labs#496)

* Update core/state_processor.go

* fix reviews

* Update precompile/contracts/txallowlist/contract_test.go

* Generalized upgrades rb nits0 (ava-labs#512)

* Minor improvements

* restore readOnly

* more updates

* Add back readOnly to allow list tests

* Precompile improvements merge (ava-labs#513)

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: Ceyhun Onur <ceyhun.onur@avalabs.org>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: cam-schultz <78878559+cam-schultz@users.noreply.github.com>
Co-authored-by: Matthew Lam <matthew.lam@avalabs.org>
Co-authored-by: cam-schultz <camschultz32@gmail.com>
Co-authored-by: omahs <73983677+omahs@users.noreply.github.com>
Co-authored-by: Anusha <63559942+anusha-ctrl@users.noreply.github.com>
Co-authored-by: Hagen Hübel <hhuebel@itinance.com>
Co-authored-by: minghinmatthewlam <matthew.lam@avalabs.org>
Fix: typos (ava-labs#428)
fix allow list comments (ava-labs#469)
fix CGO flags issue (ava-labs#489)
fix lint job (ava-labs#499)

* review fixes

* minor nits

* fix precompile generator

* fix fee manager config test

* remove debug files

* Update core/state_processor.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* fix comments

* restore statedb ordering

* fix configure in reward manager

* precompiles: adds a regression test for the IsDisabled case in AvalancheRules (ava-labs#515)

* Rename configs: alternative (ava-labs#520)

* alternative renaming for precompile configs

* fixes

* update naming

* rename to AllowListConfig

* simplify

* move blackhole check to module registerer (ava-labs#523)

* move blackhole check to module registerer

* check blackhole first

* add unit test

* Add test case for registering module outside of reserved range

---------

Co-authored-by: Aaron Buchwald <aaron.buchwald56@gmail.com>

* precompile: improve test structure (ava-labs#517)

* refactor precompile tests

* minor improvements

* nit

* fix merge

* rename package

* pr comments

* rm file

* merge AllowListTests

* pr comments

* explicit BeforeHook

* wspace

* Mark TestTransactionIndices flaky

---------

Co-authored-by: Aaron Buchwald <aaron.buchwald56@gmail.com>

* nit improvements (ava-labs#529)

* nit improvements

* move comments to README

* Update cmd/precompilegen/template-readme.md

* Rename new config (ava-labs#528)

* rename configurator's new config to make config

* use new built-in to create new config instance

* precompile: just nits (ava-labs#534)

* fix e2e comment in readme (ava-labs#540)

* fix config template for precompilegen (ava-labs#538)

* fix config template for precompilegen

* nits

* nit

* nits

* Update compatibility in README for v0.4.10 (ava-labs#542)

* Bump minimum golang version to v1.20.1 (ava-labs#548)

* Bump minimum golang version to 1.20.1

* Remove debug flag from gh action release

* add whitespace (ava-labs#544)

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Set minimum golang version to go1.19.6 (ava-labs#551)

* add custom marshaller for GetChainConfigResponse api (ava-labs#546)

* add yet another custom marshaller to GetChainConfigResponse api

* enforce a json len before allocation

* Move chain config wrapper type to params/ and add test

* Fix trailing newline

* Remove flaky unit test

* Add back unit test with JSONeq

---------

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Start coreth migration (ava-labs#552)

* Start coreth migration

* Bump version to v0.4.11 and avalanchego dep

* goimports core/blockchain.go

* Update compatibility.json

* Update compatibility in README

* Bump avalanchego dep to v1.9.10

* Start release cycle v0.4.12 (ava-labs#559)

* start v0.4.12 release cycle

* add setup action to lint ci for proper go version

* Bump version in plugin/evm/version.go

* Remove go version pin

* Revert "Remove go version pin"

This reverts commit e651beb.

* bump golangci-lint-acion to @V3

* bump golangci lint

* try 1.48

* golangci-lint v1.51 and goimports leveldb file

* migrate linting changes from coreth

* goimports and fix diagram comments

* Fix bad goimports changes

---------

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* remove stderr pipelining (ava-labs#562)

* Add ChainConfig JSON Unmarshaller (ava-labs#554)

* add unmarshaller

* add comments

* Update codeql to v2 (ava-labs#566)

* state modifications as network upgrade (ava-labs#549)

Co-authored-by: Aaron Buchwald <aaron.buchwald56@gmail.com>

* Fix Dockerfile and add Build Image to GH Actions (ava-labs#561)

* Remove specified git version to avoid downgrades breaking docker build

* Add build docker image to unit test action

* Update to use current branch instead of commit for docker image build

* Update docker image script to push image if env var is set

* Add new line to build image script

* Update scripts/constants.sh

Co-authored-by: Sam Batschelet <sam.batschelet@avalabs.org>

* Update docker login

* Update build image action

* Update gh action

* checkout for docker image build

* Set build image ID in gh action with github.ref_name

* Separate CI and release

* remove extra copy

* Update name

* Update variable used in build image id

* Update set var

* Update dockerhub repo

* Revert dockerhub repo name chnge and use Subnet-EVM in tag name

---------

Co-authored-by: Sam Batschelet <sam.batschelet@avalabs.org>

* add compatibility to readme (ava-labs#568)

* Precompile pre post handling (ava-labs#524)

* Re-apply warp precompile interface changes

* Address nits

* Separate predicate storage slot preparation into separate function in statedb

* fix lint

* improve miner enforcePredicates comment

* Add HashSliceToBytes test case for empty slice

* Address comments

* Address comments WIP

* Pre+post handling diff for shared mem precompile

* Separate proposer and general precompile predicates

* Update ShouldVerifyWithContext to return true iff proposer predicate is specified

* Add checkPredicates unit test

* Update .gitignore

* goimports

* update

* goimports config

* Address PR review comments and improve comments

* Fix typo

* Address PR comments

* Add rules into PrepareAccessList

* Only copy bytes in preparePredicates if predicate precompile is active

* Address PR comments

---------

Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>

* Warp preparation (ava-labs#573)

* Add warp precompile preparation

* Update hash slice packing

* Remove unnecessary local var

* Add VM type assertion

* Enable Warp API by default

* convert from int->uint for more clarity (ava-labs#575)

* release tickers on shutdown (ava-labs#574)

* release tickers on shutdown

* move shutdownWg.done to other defer block

* simplify functionSignatureRegex (ava-labs#578)

* Remove unused imgs from repo (ava-labs#580)

* Matches go-ethereum/pull/26912/files (ava-labs#582)

* Add ignore unnecessary import to precompile contract template (ava-labs#581)

* trie, accounts/abi: nits: adds err checks (ava-labs#583)

* Adds a test for PeerTracker (ava-labs#576)

* upgrade avalanche go and add logs

* update avalanche go version

* add gotenv and get env variables from .env file

* Revert "add gotenv and get env variables from .env file"

This reverts commit 87b0007.

* refactoring

* upgrade avalanche go to v1.9.14

* use GRPC_ prefix in env variable to inject env variables

---------

Co-authored-by: Hagen Hübel <hhuebel@itinance.com>
Co-authored-by: Ceyhun Onur <ceyhun.onur@avalabs.org>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: minghinmatthewlam <matthew.lam@avalabs.org>
Co-authored-by: Darioush Jalali <darioush.jalali@avalabs.org>
Co-authored-by: cam-schultz <78878559+cam-schultz@users.noreply.github.com>
Co-authored-by: cam-schultz <camschultz32@gmail.com>
Co-authored-by: Patrick O'Grady <prohb125@gmail.com>
Co-authored-by: Sam Batschelet <sam.batschelet@avalabs.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request precompile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants