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

feat: support all available chains #267

Merged
merged 1 commit into from
Dec 30, 2023

Conversation

zsluedem
Copy link
Collaborator

No description provided.

Copy link
Member

@Vid201 Vid201 left a comment

Choose a reason for hiding this comment

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

I see a problem with automatically adding support for every chain, specially tied to L2s: the gas for bundle is calculated differently on L2s (for example on Optimism and Arbitrum, and even between those two L2s there are differences). Users running Silius for the first time on some EVM chain could end up in weird behaviour.

That's way I think it's better to manually add supported chains on which the Silius was tested successfully.

@zsluedem
Copy link
Collaborator Author

zsluedem commented Dec 28, 2023

I see a problem with automatically adding support for every chain, specially tied to L2s: the gas for bundle is calculated differently on L2s (for example on Optimism and Arbitrum, and even between those two L2s there are differences). Users running Silius for the first time on some EVM chain could end up in weird behaviour.

That's way I think it's better to manually add supported chains on which the Silius was tested successfully.

Maybe we could just put a big warning for user that some of the chains are not officially supported and use at your own risk? @Vid201

@Vid201
Copy link
Member

Vid201 commented Dec 28, 2023

I see a problem with automatically adding support for every chain, specially tied to L2s: the gas for bundle is calculated differently on L2s (for example on Optimism and Arbitrum, and even between those two L2s there are differences). Users running Silius for the first time on some EVM chain could end up in weird behaviour.
That's way I think it's better to manually add supported chains on which the Silius was tested successfully.

Maybe we could just put a big warning for user that some of the chains are not officially supported and use at your own risk? @Vid201

Ok, let's do that. Then let's add this in the README, in the section supported chains. We can say it was tested on the chains in the table, but you can try on any other chain if you want.

@zsluedem zsluedem force-pushed the support-more-chains branch 2 times, most recently from 24ab16b to ddba60b Compare December 29, 2023 14:05
@zsluedem zsluedem requested a review from Vid201 December 29, 2023 14:05
@zsluedem zsluedem merged commit a84cb80 into silius-rs:main Dec 30, 2023
2 checks passed
@zsluedem zsluedem deleted the support-more-chains branch December 30, 2023 00:29
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

Successfully merging this pull request may close these issues.

2 participants