Skip to content

chore(eth): refactor eth API module into separate pieces in new pkg - #12796

Merged
rvagg merged 3 commits into
masterfrom
rvagg/ethmodule-refactor
Dec 20, 2024
Merged

chore(eth): refactor eth API module into separate pieces in new pkg#12796
rvagg merged 3 commits into
masterfrom
rvagg/ethmodule-refactor

Conversation

@rvagg

@rvagg rvagg commented Dec 19, 2024

Copy link
Copy Markdown
Member

As I start on the /v2 API the eth module is my first hurdle: I was /v1 to remain as it is now, but /v2 to have the same Eth endpoints but be F3 aware, so "finalized" and "safe" get adjusted according to F3. Otherwise they should be the same. Because almost everything is in a single eth.go, it makes it a bit hard to do a nice pluggable solution where I can reuse most of the code and just have replacements for a resolver. So I decided to rip apart the eth.go monolith into smaller components, in their own package, with interfaces for their dependencies rather than the concrete types.

There's very little change in the code even though the diff is big, it's mostly just moving things around. I took a few opportunities to clean up some awkward messes, and I was forced to do some new mildly awkward new things.

  • github.com/filecoin-project/lotus/node/impl/eth is a new package, with what used to be in github.com/filecoin-project/lotus/node/impl/full/eth*.go but logically split up.
  • The new eth package requires on fewer pieces of the rest of the codebase, in particular interfaces have been introduced for the main functional dependencies like ChainStore and StateManager.
  • github.com/filecoin-project/lotus/node/impl/gasutils contains some components that were in node/impl/full/gas.go but are shared between Filecoin and Eth APIs.
  • The construction and startup utilities in node/modules/ethmodule.go are now in node/modules/eth.go but have been trimmed because we need to do less manual wiring now.
  • There's some pieces I haven't touched, like the filter stuff and subscription manager.
  • Made consistent use of xerrors in the new eth package.

Comment thread node/impl/eth/events.go Fixed
Comment thread node/impl/eth/events.go Fixed
@rvagg
rvagg force-pushed the rvagg/ethmodule-refactor branch 2 times, most recently from 889a696 to 0d22219 Compare December 19, 2024 08:39
@rvagg rvagg added the skip/changelog This change does not require CHANGELOG.md update label Dec 19, 2024
@rvagg
rvagg marked this pull request as ready for review December 19, 2024 23:15
@rvagg
rvagg requested a review from Stebalien December 19, 2024 23:16
Comment thread node/modules/eth.go Outdated
Comment thread node/modules/eth.go Outdated
@rvagg

rvagg commented Dec 20, 2024

Copy link
Copy Markdown
Member Author

Additional fixes applied after a sync zoom chat with @Stebalien about this:

  • Clarified naming - everything that defines an external API has an API suffix in the name, and I've removed that from some interfaces I was using to define internal things (like ChainStore). It's much clearer now when you view it through the lens of what's intended for RPC and what's not.
  • Context naming - lctx for the lifecycle context on init.
  • Added some docs around the GatewayEthSend special-case. But also, we might look at alternative ways to improve this in the future, perhaps with some more creative/newer fx use.
  • Misc cleanups from things I noticed while walking back through the code, again.

@rvagg
rvagg force-pushed the rvagg/ethmodule-refactor branch from 8998024 to 27a1b51 Compare December 20, 2024 08:25
@rvagg rvagg removed the skip/changelog This change does not require CHANGELOG.md update label Dec 20, 2024
@rvagg
rvagg enabled auto-merge (squash) December 20, 2024 08:26
@rvagg
rvagg merged commit edfa817 into master Dec 20, 2024
@rvagg
rvagg deleted the rvagg/ethmodule-refactor branch December 20, 2024 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ☑️ Done (Archive)

Development

Successfully merging this pull request may close these issues.

3 participants