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

fix: update gas parameters for a parachain, units, and miscellaneous fixes and cleanups #6

Merged
merged 8 commits into from
May 13, 2024

Conversation

peterwht
Copy link
Collaborator

@peterwht peterwht commented May 9, 2024

No description provided.

@peterwht peterwht changed the title fix: 18 decimals, add .await, update UncheckedExtrinsic fix: update gas parameters for a parachain, units, and miscellaneous fixes and cleanups May 9, 2024
@peterwht peterwht marked this pull request as ready for review May 9, 2024 22:59
@peterwht peterwht requested a review from AlexD10S May 9, 2024 23:11
@peterwht peterwht mentioned this pull request May 10, 2024
@AlexD10S AlexD10S requested a review from Daanvdplas May 10, 2024 08:39
@AlexD10S
Copy link
Collaborator

Amazing!

@AlexD10S AlexD10S mentioned this pull request May 10, 2024
@peterwht peterwht requested a review from AlexD10S May 10, 2024 21:49
Copy link
Collaborator

@AlexD10S AlexD10S left a comment

Choose a reason for hiding this comment

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

Tested after the rebase and works!

Only doubt is about the renaming of parachain-template-runtimeto frontier-parachain-runtime in the code.
I changed to parachain-template-runtime to keep consistency with the node binary name. The binary name has to be parachain-template-node, so when we generate the Zombienet config file with pop-cli, it works.

Just my opinion about naming, rest looks great.,
If you keep the renaming here you can merge: #4 after. Otherwise just close that PR

runtime/src/lib.rs Outdated Show resolved Hide resolved
@Daanvdplas Daanvdplas self-requested a review May 11, 2024 09:03
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

I don't understand though why we change the names to frontier, and in #4 we change it back.

@AlexD10S
Copy link
Collaborator

I don't understand though why we change the names to frontier, and in #4 we change it back.

It closed the other PR to avoid more confusion. It made sense when it was opened but not after the merge or your PR where naming was changed already: https://github.com/r0gue-io/evm-parachain/pull/11/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542R32.

The binary name has to be called parachain-template-node, so when we generate the Zombienet config file with pop-cli, it works without changing anything.

For consistency I would prefer to keep parachain-template-runtime too instead of frontier-parachain-runtime

@Daanvdplas Daanvdplas self-requested a review May 11, 2024 14:33
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Change frontier-parachain to parachain-template naming

@peterwht peterwht requested review from Daanvdplas and AlexD10S May 11, 2024 17:17
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

LGTM!

@AlexD10S AlexD10S merged commit 95d4c20 into main May 13, 2024
6 checks passed
@AlexD10S AlexD10S deleted the peter/fixes branch May 13, 2024 06:50
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.

3 participants