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

Rust Portal-Hive to Hive Transition Tracker Issue #979

Closed
8 of 9 tasks
KolbyML opened this issue Jan 31, 2024 · 11 comments
Closed
8 of 9 tasks

Rust Portal-Hive to Hive Transition Tracker Issue #979

KolbyML opened this issue Jan 31, 2024 · 11 comments

Comments

@KolbyML
Copy link
Member

KolbyML commented Jan 31, 2024

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

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.

@fjl
Copy link
Collaborator

fjl commented Jan 31, 2024

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

@KolbyML
Copy link
Member Author

KolbyML commented Jan 31, 2024

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!

@fjl
Copy link
Collaborator

fjl commented Jan 31, 2024

I have changed the setting in CircleCI

@KolbyML
Copy link
Member Author

KolbyML commented Jan 31, 2024

I made an error in the formatting of the continue_circleci file in #980
here is a PR to fix that #982 now CI should work again.

@KolbyML
Copy link
Member Author

KolbyML commented Feb 1, 2024

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.

@fjl
Copy link
Collaborator

fjl commented Feb 5, 2024

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.

@KolbyML
Copy link
Member Author

KolbyML commented Feb 5, 2024

Here is a PR with the readme we had on portal-hive #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 KolbyML changed the title Portal-Hive to Hive Transition Tracker Issue Rust Portal-Hive to Hive Transition Tracker Issue Feb 5, 2024
@KolbyML
Copy link
Member Author

KolbyML commented Feb 5, 2024

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

@fjl
Copy link
Collaborator

fjl commented Feb 7, 2024

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.

@KolbyML
Copy link
Member Author

KolbyML commented Feb 8, 2024

: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 👍

@KolbyML
Copy link
Member Author

KolbyML commented Feb 8, 2024

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

@fjl fjl closed this as completed Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants