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

wallet: Improve Fee Priority Code for Clarity #9838

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Tzadiko
Copy link
Contributor

@Tzadiko Tzadiko commented Mar 16, 2025

Hello,

I'm working on #9799 . After speaking to @nahuhh , I'd like to contribute in two steps. The first step is to clean the existing code, and then addressing the issue. This makes reviewing my changeset easier.

In this changeset there are no changes to functionality. I added fee_priority .h and .cpp. Here I added the enumeration FeePriority to replace the integer values which were confusing to understand when trying to address the original issue linked above.

I will now be looking at addressing the original issue linked, working off this 'clean up' branch.

@Tzadiko
Copy link
Contributor Author

Tzadiko commented Mar 16, 2025

It built locally for me on Ubuntu, but it's failing on other distros. I'll take a look after lunch.

Done: The only distro that fails now is Arch Linux and it's unrelated to my changeset.

@Tzadiko
Copy link
Contributor Author

Tzadiko commented Mar 17, 2025

All tests pass locally. As was communicated to me (by @jeffro256 on Matrix), the test that failed here fails sporadically.

}

static FeePriority FromIntegral(const uint32_t priority)
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

if (req.priority > 4) {
LOG ERROR;
er.message = "Invalid priority value. Must be between 0 and 4.";
}

Copy link
Contributor Author

@Tzadiko Tzadiko Mar 18, 2025

Choose a reason for hiding this comment

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

Thanks, please see the commit where I add this: 41b71b9

@Tzadiko Tzadiko force-pushed the tzadiko/clean-fee-code branch from 69aefe5 to 41b71b9 Compare March 18, 2025 04:52
@Tzadiko Tzadiko changed the title Improve Fee Priority Code for Clarity wallet: Improve Fee Priority Code for Clarity Mar 18, 2025
Part of a later PR that will change the code (clean-up and change are
separate).
@Tzadiko Tzadiko force-pushed the tzadiko/clean-fee-code branch from bbd12c2 to 577fc9f Compare March 25, 2025 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants