NV18: Filecoin EVM runtime + Actor Events + EthAccount + EAM + f4 addressing #9998

raulk · 2023-01-13T01:29:00Z

Final landable version of #9617 with review comments addressed.

Related Issues

This mega-PR is the result of several months of development on the FVM Milestone 2.1 (Filecoin EVM compatibility). It includes the Lotus-side implementation of the following FIPs:

FIP-0048 (f4 address class)
FIP-0049 (Actor Events)
FIP-0054 (Filecoin EVM Runtime)
FIP-0055 (Supporting Ethereum Accounts, Addresses, and Transactions)

Proposed Changes

This 14,000 LoC PR has a ton of changes, but they broadly fall into these headlines:

The f4 address class is introduced with support for delegated addresses.
Uses a new bundle that includes the brand new EVM, EAM, and EthAccount actors.
An implementation of the Ethereum JSON-RPC API is included, mapping over to Filecoin specifics.
Added support for Actor Events (although needs an update to align with the final draft of FIP-0049 (tracked in FIP-0049 (actor events): align with implementation. FIPs#581). This includes event processing, indexing, and subscription mechanisms.
Adds support for the Wallaby testnet, although the intention is to move such support to a separate branch.
The migration for nv18 is included.
Genesis changes to instantiate the Eth Null Address at 0x0 and the EAM are also included.
An eth CLI has been implemented for diagnostics and quick experimentation.
TipsetCID generation and persistence is also implemented as per FIP-0054.
FVM v3 is integrated via filecoin-ffi.

Additional Info

There are some TODOs left over from the original review, marked with TODO::FVM. Fixing them is tracked in Fix TODO::FVM comments in Lotus ref-fvm#1446.
Should probably merge by squashing and making sure that the collective authorship is retained (I can take care of this once approved).

Checklist

Before you mark the PR ready for review, please make sure that:

Commits have a clear commit message.
PR title is in the form of of <PR type>: <area>: <change being made>
- example: fix: mempool: Introduce a cache for valid signatures
- PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
- area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
New features have usage guidelines and / or documentation updates in
- Lotus Documentation
- Discussion Tutorials
Tests exist for new functionality or change in behavior
- In progress
CI is green

Make it use GasEstimateMessageGas, which applies overestimation by default. This accounts for inclusion costs.

Also updates the actors to accommodate this change, and fix a bug in looking up addresses for f4 actors.

Co-authored-by: Raúl Kripalani <[email protected]>

We need to add full FEVM state support, but that will require merging master. This is enough for now. fixes filecoin-project/ref-fvm#1022

…it run. (#9564)

and skip a non-testing test; printing doesn't constitute testing.

Only when actors version is >= 10

it breaks the gen script in proxy_gen

it's in the actor state only if it is a v5 (or later) state tree

it's a dummy, but at least it makes tests pass

…ig + cleanups (#9970) Co-authored-by: raulk <[email protected]>

… and conversion methods (#9992)

arajasek

Let's do it.

magik6k · 2023-01-13T14:28:41Z

chain/consensus/filcns/upgrades.go

+	// Use half the CPUs for pre-migration, but leave at least 3.
+	workerCount := MigrationMaxWorkerCount
+	if workerCount <= 4 {
+		workerCount = 1
+	} else {
+		workerCount /= 2
+	}


Not too sure about the math here, doesn't look like we always have at least 3 workers

@arajasek ?

It's at least 3 workers if possible -- that is, never take more than 1, if that would doing so would leave regular processing with fewer than 3. But do take at least one always.

magik6k · 2023-01-13T14:30:17Z

chain/events/filter/event.go

+	}
+
+	// Assume short lists of addresses
+	// TODO: binary search for longer lists or restrict list length


Really easy with https://pkg.go.dev/sort#Search

magik6k · 2023-01-13T14:31:25Z

chain/events/filter/index.go

+	"PRAGMA read_uncommitted = ON",
+}
+
+var ddls = []string{


Don't we want to add at least some indexes here?

Yep, we do. @iand wanted to get a good understanding of performance before indices are added, but I am fully onboard with adding them already since it's a no-brainer.

magik6k · 2023-01-13T14:37:09Z

chain/messagepool/messagepool.go

+	// TODO: I'm not thrilled about depending on filcns here, but I prefer this to duplicating logic
+	if !filcns.IsValidForSending(nv, senderAct) {


Pretty sure one of the recently merged IPC PRs has moved this to a better place

Probably on master, not on release/v1.20.0; we'll need to reconcile once we merge release/v1.20.0 into master.

magik6k · 2023-01-13T14:41:16Z

chain/messagepool/selection.go

@@ -97,7 +97,7 @@ func (sm *selectedMessages) tryToAdd(mc *msgChain) bool {
 		sm.msgs = append(sm.msgs, mc.msgs...)
 		sm.blsLimit -= l
 		sm.gasLimit -= mc.gasLimit
-	} else if mc.sigType == crypto.SigTypeSecp256k1 {
+	} else if mc.sigType == crypto.SigTypeSecp256k1 || mc.sigType == crypto.SigTypeDelegated {


Not related to this particular line, but I feel like after the upgrade we should rename places which handle both delegated and secp messages to signed or something like that (including the field in blockheader, which we can rename because it's tuple-serialized).

Yep, Signature-bearing or something rather than Signed. (BLS message are also signed, we just aggregate their signatures when we pack them into a block).

magik6k · 2023-01-13T14:42:54Z

chain/signatures.go

@@ -0,0 +1,53 @@
+package chain


Not a fan of putting things in the chain package, can easily generate dependency loops

Ack, if that's the case we can move out. This used to be in a signatures package, but @arajasek asked to move it out. We can move it somewhere else easily.

magik6k · 2023-01-13T14:43:26Z

chain/stmgr/call.go

+			}
+		default:
+			// XXX: Hack to make sending from f099 (and others) "just work".
+			// REMOVE ME.


magik6k · 2023-01-13T14:45:00Z

chain/stmgr/stmgr.go

@@ -42,7 +42,7 @@ type StateManagerAPI interface {
 	GetPaychState(ctx context.Context, addr address.Address, ts *types.TipSet) (*types.Actor, paych.State, error)
 	LoadActorTsk(ctx context.Context, addr address.Address, tsk types.TipSetKey) (*types.Actor, error)
 	LookupID(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error)
-	ResolveToKeyAddress(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error)
+	ResolveToDeterministicAddress(ctx context.Context, addr address.Address, ts *types.TipSet) (address.Address, error)


Pretty sure this rename breaks lite mode as StateManagerAPI is also supplied from api.Gateway

raulk and others added 30 commits November 9, 2022 19:47

fix gas estimation.

78dd7a2

Make it use GasEstimateMessageGas, which applies overestimation by default. This accounts for inclusion costs.

fix eth_feeHistory.

dc5fc9d

fix: evm: use CREATE not CREATE2 for EVM contract deployment (#9553)

6337d59

Also updates the actors to accommodate this change, and fix a bug in looking up addresses for f4 actors.

fix: allow deploying from f1/f3 accounts (#9554)

ea54499

Co-authored-by: Raúl Kripalani <[email protected]>

fix: avoid returning an error when getting embryo state (#9550)

b3b9da5

We need to add full FEVM state support, but that will require merging master. This is enough for now. fixes filecoin-project/ref-fvm#1022

fix: allow EthCall.From to be nil (#9556)

49da019

Eth JSON-RPC: eth_estimateGas: fix gas estimation by actually making …

0bd5f1d

…it run. (#9564)

upgrade go-jsonrpc to v0.1.9.

672744a

fix:add UnmarshalJSON for EthNonce (#9589)

53c7d9c

api: ethrpc: fix a potential panic when querying block info (#9593)

2e5de47

go mod tidy

64798d0

fixup merge artifacts

2b23372

make gen

deea679

make docsgen-cli

448eca5

api: ethrpc: fix unit test

6d7a33f

and skip a non-testing test; printing doesn't constitute testing.

fix genesis SetupEAM

3008394

Only when actors version is >= 10

improve migration TODO comment

a2d6c8f

fix gst builtin module refs

2588b45

it breaks the gen script in proxy_gen

fix account address resolution

6e72910

it's in the actor state only if it is a v5 (or later) state tree

lint

6e149d5

fix gateway instantation for the eth api

5949307

it's a dummy, but at least it makes tests pass

lint

0ee9f9d

fix MinTimestap for null rounds

c9c53c9

Add EventsRoot field to message receipt

85df74f

actor event ffi

f660b8c

make gen

4eb94b5

Ethereum compatible actor event API

69210d0

make gen

7383ecb

upgrade go-jsonrpc to v0.1.9.

a9cfaa9

eth api: fix warts

18f2a14

raulk and others added 10 commits January 12, 2023 08:50

fix test.

892819a

fix comment.

06b40bc

address review comments.

08ae139

clear up error message.

1c7db7c

use existing variable for empty object CID.

008d04f

make gen.

c61edd1

Eth JSON-RPC: use ToFilecoinAddress() to get f4 addr and validate v s…

105a125

…ig + cleanups (#9970) Co-authored-by: raulk <[email protected]>

clean up ethtypes: rationalize ethtypes.EthAddressFromFilecoinAddress…

3ef3239

… and conversion methods (#9992)

address review comments from @magik6k on #9617 (#9997)

014d954

fix tests.

2c83992

raulk marked this pull request as ready for review January 13, 2023 01:31

raulk requested a review from a team as a code owner January 13, 2023 01:31

raulk added 2 commits January 13, 2023 01:44

change ResolveToKeyAddr => ResolveToDeterministicAddr + friends.

6de2aca

track remaining TODOs as TODO::FVM.

46a5096

raulk force-pushed the feat/nv18-fevm branch from 6845fbf to 46a5096 Compare January 13, 2023 01:44

raulk changed the base branch from master to release/v1.20.0 January 13, 2023 02:10

arajasek approved these changes Jan 13, 2023

View reviewed changes

magik6k reviewed Jan 13, 2023

View reviewed changes

raulk merged commit cdf3812 into release/v1.20.0 Jan 13, 2023

raulk deleted the feat/nv18-fevm branch January 13, 2023 19:11

raulk mentioned this pull request Jan 13, 2023

Merge Lotus branch to release branch filecoin-project/ref-fvm#1405

Closed

Fatman13 mentioned this pull request Jan 16, 2023

[venus] merge nv18 consensus / NV18 代码合并 filecoin-project/venus#5550

Closed

arajasek mentioned this pull request Jan 16, 2023

chore: merge release/v1.20.0 into master #10030

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NV18: Filecoin EVM runtime + Actor Events + EthAccount + EAM + f4 addressing #9998

NV18: Filecoin EVM runtime + Actor Events + EthAccount + EAM + f4 addressing #9998

raulk commented Jan 13, 2023 •

edited

Loading

arajasek left a comment

magik6k Jan 13, 2023

raulk Jan 13, 2023

arajasek Jan 13, 2023

magik6k Jan 13, 2023

magik6k Jan 13, 2023

raulk Jan 13, 2023

magik6k Jan 13, 2023

raulk Jan 13, 2023

magik6k Jan 13, 2023

raulk Jan 13, 2023

magik6k Jan 13, 2023

raulk Jan 13, 2023

magik6k Jan 13, 2023

raulk Jan 13, 2023

magik6k Jan 13, 2023

		// TODO: I'm not thrilled about depending on filcns here, but I prefer this to duplicating logic
		if !filcns.IsValidForSending(nv, senderAct) {

NV18: Filecoin EVM runtime + Actor Events + EthAccount + EAM + f4 addressing #9998

NV18: Filecoin EVM runtime + Actor Events + EthAccount + EAM + f4 addressing #9998

Conversation

raulk commented Jan 13, 2023 • edited Loading

Related Issues

Proposed Changes

Additional Info

Checklist

arajasek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raulk commented Jan 13, 2023 •

edited

Loading