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: add FeeTier support to galileo tx sending #98

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented Jun 4, 2024

During Testnet 77 we enabled fees [0] and in the process broke some things like Galileo. Adding FeeTier support is rather straightforward, although this changeset requires a patch to the penumbra-fee crate in order to compile [1].

[0] penumbra-zone/penumbra#4306
[1] penumbra-zone/penumbra#4539

@conorsch conorsch requested a review from cratelyn June 4, 2024 16:43
@cratelyn cratelyn added this to the Sprint 8 milestone Jun 4, 2024
@cratelyn cratelyn marked this pull request as draft June 4, 2024 16:53
@cratelyn
Copy link
Contributor

cratelyn commented Jun 4, 2024

ci will fail until the corresponding monorepo PR is merged. iirc galileo CI runs against both main and the most recent tag. that PR will work on main, giving us confidence that it's compatible with preview/next, but will still fail against 0.77.1, because 0.77.1 lacks the impls it needs

quoth @conorsch, in #review-please.

there's still a bit of fee plumbing to do, so i've marked this as a draft, but will review once it's ready 🙂

@cratelyn cratelyn removed their request for review June 4, 2024 17:03
Copy link
Contributor

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

this looks good to me! 🙏

justfile Outdated Show resolved Hide resolved
@conorsch conorsch marked this pull request as ready for review June 4, 2024 18:41
@conorsch
Copy link
Contributor Author

conorsch commented Jun 4, 2024

Closes #97. Tested by running locally and pasting an address into a private discord channel: received funds.

Copy link
Contributor

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

thanks for removing the fee flag altogether ✨

During Testnet 77 we enabled fees [0] and in the process broke some
things like Galileo. Adding FeeTier support is rather straightforward,
although this changeset requires a patch to the `penumbra-fee` crate in
order to compile [1].

[0] penumbra-zone/penumbra#4306
[1] penumbra-zone/penumbra#4539
@cratelyn cratelyn merged commit ec84f80 into main Jun 4, 2024
5 checks passed
@cratelyn cratelyn mentioned this pull request Jun 4, 2024
@conorsch
Copy link
Contributor Author

conorsch commented Jun 4, 2024

Confirmed this changeset still works after building against penumbra-zone/penumbra#4559

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants