-
Notifications
You must be signed in to change notification settings - Fork 3.4k
fix: remove negative fee assert from get_tx_fee_warning #10073
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
Open
f321x
wants to merge
1
commit into
spesmilo:master
Choose a base branch
from
f321x:fix_issue_10065
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
why would a psbt have negative fee? |
Member
Author
|
Maybe in some collaborative setup where psbts get combined and another psbt brings additional inputs for the fee? Don't know of a specific use case but throwing an exception seems strict as long as the psbt is otherwise "valid". |
f321x
added a commit
to f321x/electrum-fork
that referenced
this pull request
Jul 22, 2025
It seems like the OS version of the CI image has been bumped, the regtest of PR spesmilo#10073 fails for me with this error: ``` (Reading database ... 55% (Reading database ... 60% (Reading database ... 65% (Reading database ... 70% (Reading database ... 75% (Reading database ... 80% (Reading database ... 85% (Reading database ... 90% (Reading database ... 95% (Reading database ... 100% (Reading database ... 111702 files and directories currently installed.) Preparing to unpack .../jq_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking jq (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Preparing to unpack .../libjq1_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Setting up libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) ... Setting up jq (1.7.1-3ubuntu0.24.04.1) ... Processing triggers for man-db (2.12.0-4build2) ... Processing triggers for libc-bin (2.39-0ubuntu8.5) ... Running kernel seems to be up-to-date. No services need to be restarted. No containers need to be restarted. No user sessions are running outdated binaries. No VM guests are running outdated hypervisor (qemu) binaries on this host. python3 -m pip install --user --upgrade pip error: externally-managed-environment × This environment is externally managed ╰─> To install Python packages system-wide, try apt install python3-xyz, where xyz is the package you are trying to install. If you wish to install a non-Debian-packaged Python package, create a virtual environment using python3 -m venv path/to/venv. Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make sure you have python3-full installed. If you wish to install a non-Debian packaged Python application, it may be easiest to use pipx install xyz, which will manage a virtual environment for you. Make sure you have pipx installed. See /usr/share/doc/python3.12/README.venv for more information. note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages. hint: See PEP 668 for the detailed specification. ``` This should make the pip installs work again, however not sure how to test it *on* the CI? i it locally and it seems to work on Ubuntu 24.04 with this patch.
f321x
added a commit
to f321x/electrum-fork
that referenced
this pull request
Jul 22, 2025
It seems like the OS version of the CI image has been bumped, the regtest of PR spesmilo#10073 fails for me with this error: ``` (Reading database ... 55% (Reading database ... 60% (Reading database ... 65% (Reading database ... 70% (Reading database ... 75% (Reading database ... 80% (Reading database ... 85% (Reading database ... 90% (Reading database ... 95% (Reading database ... 100% (Reading database ... 111702 files and directories currently installed.) Preparing to unpack .../jq_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking jq (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Preparing to unpack .../libjq1_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Setting up libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) ... Setting up jq (1.7.1-3ubuntu0.24.04.1) ... Processing triggers for man-db (2.12.0-4build2) ... Processing triggers for libc-bin (2.39-0ubuntu8.5) ... Running kernel seems to be up-to-date. No services need to be restarted. No containers need to be restarted. No user sessions are running outdated binaries. No VM guests are running outdated hypervisor (qemu) binaries on this host. python3 -m pip install --user --upgrade pip error: externally-managed-environment × This environment is externally managed ╰─> To install Python packages system-wide, try apt install python3-xyz, where xyz is the package you are trying to install. If you wish to install a non-Debian-packaged Python package, create a virtual environment using python3 -m venv path/to/venv. Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make sure you have python3-full installed. If you wish to install a non-Debian packaged Python application, it may be easiest to use pipx install xyz, which will manage a virtual environment for you. Make sure you have pipx installed. See /usr/share/doc/python3.12/README.venv for more information. note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages. hint: See PEP 668 for the detailed specification. ``` This should make the pip installs work again, however not sure how to test it *on* the CI? i it locally and it seems to work on Ubuntu 24.04 with this patch.
f321x
added a commit
to f321x/electrum-fork
that referenced
this pull request
Jul 22, 2025
It seems like the OS version of the CI image has been bumped, the regtest of PR spesmilo#10073 fails for me with this error: ``` (Reading database ... 55% (Reading database ... 60% (Reading database ... 65% (Reading database ... 70% (Reading database ... 75% (Reading database ... 80% (Reading database ... 85% (Reading database ... 90% (Reading database ... 95% (Reading database ... 100% (Reading database ... 111702 files and directories currently installed.) Preparing to unpack .../jq_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking jq (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Preparing to unpack .../libjq1_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Setting up libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) ... Setting up jq (1.7.1-3ubuntu0.24.04.1) ... Processing triggers for man-db (2.12.0-4build2) ... Processing triggers for libc-bin (2.39-0ubuntu8.5) ... Running kernel seems to be up-to-date. No services need to be restarted. No containers need to be restarted. No user sessions are running outdated binaries. No VM guests are running outdated hypervisor (qemu) binaries on this host. python3 -m pip install --user --upgrade pip error: externally-managed-environment × This environment is externally managed ╰─> To install Python packages system-wide, try apt install python3-xyz, where xyz is the package you are trying to install. If you wish to install a non-Debian-packaged Python package, create a virtual environment using python3 -m venv path/to/venv. Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make sure you have python3-full installed. If you wish to install a non-Debian packaged Python application, it may be easiest to use pipx install xyz, which will manage a virtual environment for you. Make sure you have pipx installed. See /usr/share/doc/python3.12/README.venv for more information. note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages. hint: See PEP 668 for the detailed specification. ``` This should make the pip installs work again, however not sure how to test it *on* the CI? i it locally and it seems to work on Ubuntu 24.04 with this patch.
f321x
added a commit
to f321x/electrum-fork
that referenced
this pull request
Jul 22, 2025
It seems like the OS version of the CI image has been bumped, the regtest of PR spesmilo#10073 fails for me with this error: ``` (Reading database ... 55% (Reading database ... 60% (Reading database ... 65% (Reading database ... 70% (Reading database ... 75% (Reading database ... 80% (Reading database ... 85% (Reading database ... 90% (Reading database ... 95% (Reading database ... 100% (Reading database ... 111702 files and directories currently installed.) Preparing to unpack .../jq_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking jq (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Preparing to unpack .../libjq1_1.7.1-3ubuntu0.24.04.1_amd64.deb ... Unpacking libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) over (1.7.1-3build1) ... Setting up libjq1:amd64 (1.7.1-3ubuntu0.24.04.1) ... Setting up jq (1.7.1-3ubuntu0.24.04.1) ... Processing triggers for man-db (2.12.0-4build2) ... Processing triggers for libc-bin (2.39-0ubuntu8.5) ... Running kernel seems to be up-to-date. No services need to be restarted. No containers need to be restarted. No user sessions are running outdated binaries. No VM guests are running outdated hypervisor (qemu) binaries on this host. python3 -m pip install --user --upgrade pip error: externally-managed-environment × This environment is externally managed ╰─> To install Python packages system-wide, try apt install python3-xyz, where xyz is the package you are trying to install. If you wish to install a non-Debian-packaged Python package, create a virtual environment using python3 -m venv path/to/venv. Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make sure you have python3-full installed. If you wish to install a non-Debian packaged Python application, it may be easiest to use pipx install xyz, which will manage a virtual environment for you. Make sure you have pipx installed. See /usr/share/doc/python3.12/README.venv for more information. note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages. hint: See PEP 668 for the detailed specification. ``` This should make the pip installs work again, however not sure how to test it *on* the CI? i it locally and it seems to work on Ubuntu 24.04 with this patch.
rm the `assert fee >= 0, f"{fee=!r} must be non-negative satoshis"`
from `Abstract_Wallet.get_tx_fee_warning()` to prevent an exception when
users load a psbt with negative tx fee.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removes the
assert fee >= 0, f"{fee=!r} must be non-negative satoshis"fromAbstract_Wallet.get_tx_fee_warning()to prevent an exception when users load a psbt with negative tx fee. Is there any reason this would be strictly required?get_tx_fee_warning()handles the negative fee by showing an error that the fee is below the relay fee, which is technically correct. Maybe the user wants to add inputs with a second psbt.This signet psbt can be used to trigger the exception:
Fixes #10065