Remove amount assert in pjuri fuzzer#1480
Conversation
Coverage Report for CI Build 24461549759Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 84.34%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Until a decision can be made regarding payjoin/bitcoin_uri#11 we should not need to check if the amount is greater than max amount.
benalleng
force-pushed
the
max-amount-fuzzer
branch
from
19ba333 to
85b73f2
Compare
xstoicunicornx
left a comment
xstoicunicornx
left a comment
There was a problem hiding this comment.
Ack 85b73f2
Removing the amount assert from pj uri fuzzer until there is clarity on payjoin/bitcoin_uri#11 makes sense to me. Reviewed changes and ran fuzzer.
| // Remove assert for max amount fuzzer as long as we think its not necessary | ||
| // assert!(amount.is_none_or(|btc| btc < Amount::MAX_MONEY)); |
There was a problem hiding this comment.
Nit: Maybe don't need these comments at all?
I get that the comments are meant to keep payjoin/bitcoin_uri#11 resolution top of mind, and agree that the comments help with that, but it made me ask why fuzzing of bitcoin_uri is happening in this repo rather than in bitcoin_uri repo?
There was a problem hiding this comment.
This was added mostly as a POC when installing the fuzzer in payjoin but it gets run whenever I do a .cycle.sh run so I catch stuff in my test occasionally.
Ideally over the next year I'd like to pull it out and move it over to bitcoin_uri as we get more actual payjoin related fuzzers
The relevant asserts and questions can be found in bitcoin_uri and the history of this file.
4 participants
Until a decision can be made regarding payjoin/bitcoin_uri#11 we should not need to check if the amount is greater than max amount.
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.