feat: ethrpc: implement EthBlock and EthTx structs #9287

ychiaoli18 · 2022-09-12T01:51:57Z

Related Issues

ref-fvm #854

Proposed Changes

implement EthBlock and EthTx structs
implement the following methods
- eth_getBlockByHash
- eth_getBlockByNumber
- eth_getTransactionByHash

Additional Info

Checklist

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

All commits have a clear commit message.
The 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, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, test
- area: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps
This PR has tests for new functionality or change in behaviour
If new user-facing features are introduced, clear usage guidelines and / or documentation updates should be included in https://lotus.filecoin.io or Discussion Tutorials.
CI is green

vyzo

looks fine for now.

vyzo · 2022-09-12T17:50:17Z

api/eth_types.go

+)
+
+var (
+	// TODO: need to determine based on build params


put this in build/params to do this.

raulk · 2022-09-12T11:35:03Z

api/eth_types.go

+const (
+	CHAIN_ID_MAINNET     = 314
+	CHAIN_ID_BUILDERNET  = 3141
+	CHAIN_ID_WALLABY     = 31415
+	CHAIN_ID_CALIBRATION = 314159
+	CHAIN_ID_BUTTERFLY   = 3141592
+	CHAIN_ID_LOCAL       = 31415926
+)
+
+var (
+	// TODO: need to determine based on build params
+	CHAIN_ID_CURRENT = CHAIN_ID_MAINNET


We need to solve this before merge. Should be trivial? Just add the appropriate const to the params_ file for the corresponding network?

raulk · 2022-09-12T11:36:10Z

node/impl/full/eth.go

+		From:                 fromEthAddr,
+		To:                   toEthAddr,
+		Value:                api.EthBigInt(msg.Value),
+		Type:                 api.EthInt(2),


I assume this indicates it's an EIP-1559 tx -- we should add a constant for this.

Yes, 2 means it's an EIP-1559 tx

raulk · 2022-09-12T17:50:11Z

api/eth_types.go

+const (
+	CHAIN_ID_MAINNET     = 314
+	CHAIN_ID_BUILDERNET  = 3141
+	CHAIN_ID_WALLABY     = 31415
+	CHAIN_ID_CALIBRATION = 314159
+	CHAIN_ID_BUTTERFLY   = 3141592
+	CHAIN_ID_LOCAL       = 31415926
+)
+
+var (
+	// TODO: need to determine based on build params
+	CHAIN_ID_CURRENT = CHAIN_ID_MAINNET


We'll want these consts to be defined in build/params_{network}.go files.

raulk · 2022-09-12T17:51:40Z

api/eth_types.go

+	replaced := strings.Replace(s, "0x", "", -1)
+	if len(replaced)%2 == 1 {
+		replaced = "0" + replaced
+	}
+
+	i := new(mathbig.Int)
+	i.SetString(replaced, 16)
+
+	*e = EthBigInt(big.NewFromGo(i))
+	return nil


Any reason why not to use the encoding/hex package? https://pkg.go.dev/encoding/hex

It's likely that the underlying string is a big int, so it's easier to parse the string directly using the standard big.Int library.

raulk · 2022-09-12T17:52:51Z

api/eth_types.go

+	type TempEthCall EthCall
+	var params TempEthCall


Why is this type dance necessary?

Otherwise decoding with json.Unmarshal will create an infinite loop and cause stack overflow. (json.Unmarshal -> EthCall.UnmarshalJSON -> json.Unmarshal -> EthCall.UnmarshalJSON -> ...)

raulk · 2022-09-12T17:56:14Z

node/impl/full/eth.go

+	for _, blkMsg := range blkMsgs {
+		for _, msg := range append(blkMsg.BlsMessages, blkMsg.SecpkMessages...) {
+			msgLookup, err := a.StateAPI.StateSearchMsg(ctx, types.EmptyTSK, msg.Cid(), api.LookbackNoLimit, true)
+			if err != nil {
+				return api.EthBlock{}, nil
+			}
+			gasUsed += msgLookup.Receipt.GasUsed
+
+			if fullTxInfo {
+				tx, err := a.ethTxFromFilecoinMessageLookup(ctx, msgLookup)
+				if err != nil {
+					return api.EthBlock{}, nil
+				}
+				block.Transactions = append(block.Transactions, tx)
+			} else {
+				hash, err := api.EthHashFromCid(msg.Cid())
+				if err != nil {
+					return api.EthBlock{}, err
+				}
+				block.Transactions = append(block.Transactions, hash.String())
+			}
+		}
+	}


I'm not sure there's any other way of doing it other than loading every single receipt and adding the gas used, like you're doing here. The problem is two-fold: (a) deferred execution due to tipsets, (b) execution tipset does not broadcast the total gas used in the chain data structure (like the Ethereum block does).

@arajasek ideas here would be welcome.

raulk · 2022-09-12T17:57:10Z

node/impl/full/eth.go

+	fromEthAddr, err := api.EthAddressFromFilecoinIDAddress(fromFilIdAddr)
+	if err != nil {
+		return api.EthTx{}, err
+	}
+
+	toFilAddr, err := a.StateAPI.StateLookupID(ctx, msg.From, types.EmptyTSK)
+	if err != nil {
+		return api.EthTx{}, err
+	}
+
+	toEthAddr, err := api.EthAddressFromFilecoinIDAddress(toFilAddr)
+	if err != nil {
+		return api.EthTx{}, err
+	}


In the future we will want these addresses to preferentially be f4 addresses when there is one.

raulk · 2022-09-12T17:57:36Z

node/impl/full/eth.go

+	}
+
+	tx := api.EthTx{
+		ChainID:              api.EthInt(api.CHAIN_ID_CURRENT),


Once you move this to build/params_{network}.go, you won't have to track the current chain ID here.

raulk

This is good to merge and iterate on on the experimental branch. We will still make quite a few changes, and the full JSON-RPC API isn't expected until Iron r04.

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

ychiaoli18 added 2 commits September 11, 2022 11:24

feat: ethrpc: implement block and tx structs

09c16ee

define the interface for eth_call and eth_estimateGas

75a968c

ychiaoli18 requested a review from a team as a code owner September 12, 2022 01:51

fix

c93b1ce

maciejwitowski mentioned this pull request Sep 12, 2022

M2.1: Copper release (r3) filecoin-project/ref-fvm#830

Closed

vyzo reviewed Sep 12, 2022

View reviewed changes

api/eth_types.go Outdated

)

var (

// TODO: need to determine based on build params

Copy link

Contributor

vyzo Sep 12, 2022

Choose a reason for hiding this comment

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

put this in build/params to do this.

raulk reviewed Sep 12, 2022

View reviewed changes

raulk and others added 4 commits September 12, 2022 19:38

set ChainID in build/params_{network}.go files.

2945e62

implement eth_chainId.

9f14249

make it compile.

005e9b9

fix lint

e36ad15

raulk approved these changes Sep 12, 2022

View reviewed changes

raulk merged commit 8bbc55e into experimental/fvm-m2 Sep 12, 2022

raulk deleted the kevin/feat/ethrpc-block-tx branch September 12, 2022 21:46

raulk mentioned this pull request Sep 12, 2022

Lotus release hand-off: FVM Copper (r03) filecoin-project/testnet-wallaby#6

Closed

Fatman13 mentioned this pull request Sep 16, 2022

[fvm] ETHblock和ETHTx相关实现? filecoin-project/venus#5299

Closed

1 task

adlrocha referenced this pull request in consensus-shipyard/lotus Oct 13, 2022

feat: ethrpc: implement EthBlock and EthTx structs (#9287)

cf6a506

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

vyzo pushed a commit that referenced this pull request Nov 9, 2022

feat: ethrpc: implement EthBlock and EthTx structs (#9287)

e5bb5b7

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

simlecode mentioned this pull request Dec 14, 2022

feat: ethrpc: implement EthBlock and EthTx structs filecoin-project/venus#5564

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ethrpc: implement EthBlock and EthTx structs #9287

feat: ethrpc: implement EthBlock and EthTx structs #9287

ychiaoli18 commented Sep 12, 2022

vyzo left a comment

vyzo Sep 12, 2022

raulk Sep 12, 2022

raulk Sep 12, 2022

ychiaoli18 Sep 12, 2022 •

edited

Loading

raulk Sep 12, 2022

raulk Sep 12, 2022

ychiaoli18 Sep 12, 2022

raulk Sep 12, 2022

ychiaoli18 Sep 12, 2022

raulk Sep 12, 2022

raulk Sep 12, 2022

raulk Sep 12, 2022

raulk Sep 12, 2022

raulk left a comment

feat: ethrpc: implement EthBlock and EthTx structs #9287

feat: ethrpc: implement EthBlock and EthTx structs #9287

Conversation

ychiaoli18 commented Sep 12, 2022

Related Issues

Proposed Changes

Additional Info

Checklist

vyzo 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

ychiaoli18 Sep 12, 2022 • edited Loading

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 left a comment

Choose a reason for hiding this comment

ychiaoli18 Sep 12, 2022 •

edited

Loading