#Issue 671: Feature request: WALLET_MAX_FEE_PPK flag to avoid overpaying fees #786
Conversation
…I guess the other tests might not work any more
|
Ready for review I'm not sure if I should tag somebody (@callebtc ?) or add a label or something to say that this is ready for review. This is my first open-source PR, so maybe I'm doing everything wrong here 😀 |
| # Apply override environment variables at the end | ||
| # This allows OVERRIDE_* env vars to override .env file settings | ||
| for key, value in os.environ.items(): | ||
| if key.startswith("OVERRIDE_"): | ||
| # Remove OVERRIDE_ prefix and set the environment variable | ||
| actual_key = key[9:] # Remove "OVERRIDE_" (9 characters) | ||
| os.environ[actual_key] = value | ||
|
|
There was a problem hiding this comment.
Was this the only way you could run a mint with fees to test the wallet?
There was a problem hiding this comment.
I must admit I was hacking a lot of different things, and it's very possible that there is a simpler solution that I missed.
Was this the only way you could run a mint with fees to test the wallet?
Now that I look at bit more at the other tests, maybe I could copy a test from tests/mint/test_mint_fees.py and use that instead. For example, I guess I could make a test that uses this Wallet and Ledger approach.
I''ll try that this evening, if I have time
There was a problem hiding this comment.
PS: Partial good news. Just a moment ago, I pushed a single commit with a simple new test. If the max_input_fee_ppk argument is too low then, as expected, it raises an Exception to complain about the fees being too high
But, when the max_input_fee_ppk argument is large enough then it should success but instead generates a strange exception: FAILED tests/wallet/test_wallet_fees.py::test_send_with_input_fee_limit - Exception: Mint Error: inputs (64) - fees (0) vs outputs (59) are not balanced. (Code: 11000)
The fees are 5 sats, so why does this error say 0 sats?
If this can be resolved, then I think the complex/ugly second mint can be removed from this PR
| server.stop() | ||
|
|
||
|
|
||
| @pytest.fixture(scope="session") |
There was a problem hiding this comment.
Was this necessary to add?
|
@aaronmcdaid please excuse the delay. Thank you for this PR, and nice tests. I wonder if it is necessary to do the OVERRIDE_ and to set up a new mint in conftest for this. Maybe you can clarify that a bit. |
Thanks for the nice feedback. I'm planning to attend the e-cash hackday in Berlin tomorrow; I've registered, but I'm not sure if there is some registration confirmation that I need? Maybe we could discuss this stuff then Although, of course, I'll try to be clearer here too. I'll respond now in-place in this PR, responding to the comments you have already made elsewhere in this PR |
… mint. However, it generates a strange error
2 participants
Update: 2025-09-21: managed to add a few tests, by adding a secondary test mint
This is a PR to implement #671: Feature request: WALLET_MAX_FEE_PPK flag to avoid overpaying fees
This adds a
--max-input-fee-ppkargument for three commands:pay: adjusts the call toselect_to_sendsuch that it will raiseInputFeeExceedsLimitErrorif theinput_fee_ppkfees imposed by the mint are large.send: adjusts the call tosendsuch that it will raiseInputFeeExceedsLimitErrorif theinput_fee_ppkfees imposed by the mint are large.selfpay: adjusts the call toselect_to_sendsuch that it will raiseInputFeeExceedsLimitErrorif theinput_fee_ppkfees imposed by the mint are large.====
The most complex thing here is the tests. The standard test mint doesn't charge any fees. If I change the fee settings of that mint, then other tests break. So instead, this PR add a secondary test mint defined in
conftest.pyto by identical to the standard test mint but with a few overrides. I was trying lots of other things, like trying (unsuccessfully) to force the mint to create a new high-fee keyset and temporarily deactivate the original no-fee keyset, but I couldn't get it to work===
Smaller comments/questions:
input_fee_ppkwhich is imposed by the mint for each token. Should there be a similar limit for other kinds of fees, such as Lightning fees?.env, instead of requiring the user to put this arg on the command line?ruffrequired some smaller cosmetic changes in lines that I didn't really touch