hive icon indicating copy to clipboard operation
hive copied to clipboard

Rust Portal-Hive to Hive Transition Tracker Issue

Open KolbyML opened this issue 1 year ago • 11 comments

Back in October Felix proposed the idea of us merging portal-hive back into hive. Here is a tracker PR which will track the progress of it. Originally I made 1 big PR, but looking back on that it probably wasn't the best idea and could be seen as overwhelming to review. So I am back with a different attept in making a more gradual transition.

PR's to complete transition of the Rust Portal Hive code

  • [x] Add portal client definitions https://github.com/ethereum/hive/pull/977
  • [x] Add ci for portal simulators https://github.com/ethereum/hive/pull/980
  • [x] Add portal history/rpc-compat simulator https://github.com/ethereum/hive/pull/978
  • [x] Add portal history/portal-interop simulator https://github.com/ethereum/hive/pull/987
  • [x] Add portal history/portal-mesh simulator https://github.com/ethereum/hive/pull/988
  • [x] Add portal history/portal-bridge simulator https://github.com/ethereum/hive/pull/989
  • [x] Add portal beacon/rpc-compat simulator https://github.com/ethereum/hive/pull/990
  • [ ] Add portal (simulator which is made during this transition period in the portal-hive repo) simulator (PR to be made)
  • [ ] Add hivesim-rs https://github.com/ethereum/hive/pull/1126

Then one day after the above is done and hivesim-rs is "stable" we can transition that too.

I can't wait till this happens as I want to write some EL -> portal-bridge -> portal client tests.

KolbyML avatar Jan 31 '24 01:01 KolbyML

Hey @KolbyML, it's actually np to have the big PR, I'm just very slow.

fjl avatar Jan 31 '24 11:01 fjl

With the merge of #980 it depends on turning on Use of setup workflows must be enabled in project settings (Project settings > Advanced -> Dynamic config using setup workflows) in circleci settings. That PR was based off this circleci documentation https://github.com/CircleCI-Public/api-preview-docs/blob/path-filtering/docs/path-filtering.md#setup-workflows

No worries. :D I am just excited with the smaller PR way I actually have something merged now which is so cool!

KolbyML avatar Jan 31 '24 13:01 KolbyML

I have changed the setting in CircleCI

fjl avatar Jan 31 '24 18:01 fjl

I made an error in the formatting of the continue_circleci file in #980 here is a PR to fix that https://github.com/ethereum/hive/pull/982 now CI should work again.

KolbyML avatar Jan 31 '24 18:01 KolbyML

The Rust CI works and only runs on the filtered path very cool. I am going to open the PR's for the other simulators tomorrow. I think it is nice with this split approach because it is also a lot less work I realized on my end to maintain the PR, with any changes we make on portal-hive in the mean time during the transitionary period.

KolbyML avatar Feb 01 '24 02:02 KolbyML

I have merged the simulators as-is. However, I think there are some things that could be improved:

  • The simulator names are very long. It might be better to remove one directory level and call them simulators/portal/history-{rpc,interop,mesh,bridge} etc.
  • I've added a 'portal' role to the clients. Don't know if hivesim-rs supports it, but hive clients have roles which you will get through the client-types response: https://github.com/ethereum/hive/blob/master/docs/simulators.md#getting-available-client-types The idea with roles is that, if you run a simulator with all available clients, it will pick the ones that it can actually use.

fjl avatar Feb 05 '24 21:02 fjl

Here is a PR with the readme we had on portal-hive https://github.com/ethereum/hive/pull/991

I would be down to remove the portal-prefix from interop/mesh/bridge and compat suffix from rpc

I would prefer simulators/portal/history/{rpc,interop,mesh,bridge} Because as we add more networks there will be simulators which use multiple networks. E.x. simulators which test the interopibility of the beacon and history network. beacon-history/rpc or beacon-history-rpc. I like beacon-history/rpc because then as we add more simulators which could possibly use multiple networks they are contained in a folder specifying networks used, instead of a long list of misc simulaters in the explorer.

If you want (networks used)-(simulator name) I am fine with that, it might just get messy as we add simulators for testing the interopibility of multiple portal networks working together.

I've added a 'portal' role to the clients. Don't know if hivesim-rs supports it, but hive clients have roles which you will get through the client-types response:

I will look into this and resolve any issues if they exist

KolbyML avatar Feb 05 '24 21:02 KolbyML

Another note is trin-bridge can only be used with the portal-bridge simulator and is required for portal-bridge simulator to be ran

KolbyML avatar Feb 05 '24 22:02 KolbyML

Looking at the simulators again, I also noticed the actual suite implementations are kind of small. One thing to note is that there is no one-to-one correspondence between simulators and test suites. You can run multiple suites from one simulator, so we could also just add a portal/history simulator that contains all of the history network test suites. This would also cut down the amount of Dockerfiles and Rust projects.

fjl avatar Feb 07 '24 16:02 fjl

:D I didn't even know that was possible. I think that is a great idea! I haven't read the simulators written in go-lang as they were never on ethereum/portal-hive at the time I started to write tests for it we just had 2 different simulators rpc-compat and portal-interop. I will read one of the go simulators to get a better idea on the best practices for structuring simulators and make a PR with the change you suggested :+1:

KolbyML avatar Feb 08 '24 19:02 KolbyML

Here https://github.com/ethereum/hive/pull/994/files here is a PR with the requested layout :D

KolbyML avatar Feb 08 '24 23:02 KolbyML