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

wallet2: adjust fee during backlog, fix set priority #9219

Merged
merged 1 commit into from Mar 9, 2024

Conversation

selsta
Copy link
Collaborator

@selsta selsta commented Mar 7, 2024

Resolves #9221

The automatic fee level should switch between slow and normal depending on the current backlog.

https://monero.stackexchange.com/questions/11657/what-are-the-standard-fee-multiplier-levels#comment10765_11658

This behaviour broke at some point, likely during the 2021 fee code changes. This PR restores the previous behaviour.

@jeffro256
Copy link
Contributor

I don't think that I agree with these changes. Sometimes, one knows that there's a backlog, but trusts that their transaction will make it into the block in an acceptable timeframe. Forcing the priority to rise because of a backlog gives attackers more leverage to disrupt the network.

@SamsungGalaxyPlayer
Copy link
Collaborator

SamsungGalaxyPlayer commented Mar 7, 2024

The intent of the automatic feature has always been to alternate between the slow and normal fee levels as appropriate. I recommend that unless there's a really strong reason to completely change the logic behind these fee level adjustments, the discrepancy versus the expected behavior is addressed.

@selsta
Copy link
Collaborator Author

selsta commented Mar 7, 2024

@jeffro256 the current code is broken since the 2021 fee changes. This PR fixes that back to the previous behavior.

Also in your example couldn't you manually select the lowest fee instead of auto?

@LocalMonero
Copy link
Contributor

This returns the expected behavior. The mempool spam would be much less noticeable by common users had this bug not been introduced in 2021. Thanks @selsta for fixing it so quickly, and @luigi1111 please merge this urgently and release the new binaries for everyone.

@selsta
Copy link
Collaborator Author

selsta commented Mar 8, 2024

Should I also change the remaining return priority; to either 1 or 2?

Copy link

@nahuhh nahuhh left a comment

Choose a reason for hiding this comment

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

LGTM

anonero wallet's @r4v3r23 pulled pr and tested on anonero.

@r4v3r23
Copy link

r4v3r23 commented Mar 8, 2024

standard tx fees are now ~4x the size than previous code's behavior

+1 from me

@osensei
Copy link
Contributor

osensei commented Mar 8, 2024

So, I've done some tests with these changes, and it seems to work as expected ONLY when wallet's settings are:
priority = 0 AND auto-low-priority = 1

In any other case the priority setting seems to be ignored, and will always send with the minimum fee, even if you set priority = 4 AND auto-low-priority = 0. This also happens on v0.18.3.1.

In those other cases, the only way to override the minimum fee is to specify the priority in the transfer command, .e.g.:
transfer normal <address> <amount>

@osensei
Copy link
Contributor

osensei commented Mar 8, 2024

In any other case the priority setting seems to be ignored, and will always send with the minimum fee, even if you set priority = 4 AND auto-low-priority = 0. This also happens on v0.18.3.1.

I understand that my observations may not apply directly to this PR, but just wanted to state the facts, as it seemed relevant. Also, I just saw that @selsta opened #9221 related to this.

@SChernykh
Copy link
Contributor

SChernykh commented Mar 8, 2024

In any other case the priority setting seems to be ignored, and will always send with the minimum fee, even if you set priority = 4 AND auto-low-priority = 0. This also happens on v0.18.3.1.

See my comment there. #9219 should fix it too.

Edit: uhh, I was probably wrong and #9221 will probably require a separate fix.

@selsta selsta changed the title wallet2: automatically adjust fee during backlog wallet2: adjust fee during backlog, fix set priority Mar 8, 2024
@luigi1111 luigi1111 merged commit 769202b into monero-project:master Mar 9, 2024
18 checks passed
@selsta selsta deleted the adjust-priority branch March 9, 2024 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

set priority appears broken
9 participants