fix(receive): Enforce BIP-352 K_MAX per-group recipient limit in scan_txouts - #75
Merged
Merged
Conversation
nymius
reviewed
Sep 1, 2026
MusabYK
force-pushed
the
add-k_max-cap
branch
2 times, most recently
from
September 2, 2026 19:13
e319f66 to
345a109
Compare
nymius
requested changes
Sep 2, 2026
nymius
left a comment
nymius
left a comment
Collaborator
There was a problem hiding this comment.
Can you squash both commits together? Also, in general, I prefer commit messages that explain the what and why of the code, rather than short descriptions.
MusabYK
force-pushed
the
add-k_max-cap
branch
from
September 3, 2026 05:51
345a109 to
dfd71b8
Compare
Contributor
Author
|
Hi @nymius I squashed both commits together and updated the commit message to include the full context. Thanks for the feedback |
nymius
reviewed
Sep 3, 2026
MusabYK
force-pushed
the
add-k_max-cap
branch
from
September 3, 2026 13:25
dfd71b8 to
1598b06
Compare
Contributor
Author
Done! |
nymius
approved these changes
Sep 3, 2026
Collaborator
|
If you want to work in a follow up: I prefer these slow tests to only execute on CI, and ignore them in |
Contributor
Author
|
Makes sense. I'll look into it. |
8 tasks
nymius
added a commit
that referenced
this pull request
Sep 7, 2026
…, still run them in CI 6d61ce3 refactore: ignore slow tests locally during pre-push (MusabYK) Pull request description: <!-- You can erase any parts of this template not applicable to your Pull Request. --> ### Description This is a follow up #75 ### Notes to the reviewers I group slow but correct/implemented tests under a nested mod slow { .. } block and add #[ignore]. This excludes them from local cargo test runs (just test/pre-push), but CI targets them all using a single filter: cargo test -- --ignored slow::. slow tests should now be added in mod slow{..} block in any file or crate. Also added `just test-slow` (alias just ts) and `just test-all` (alias just ta) since slow tests are still real tests someone may want to run before trusting a change. ### Changelog notice <!-- Notice the release manager should include in the release tag message changelog --> <!-- See https://keepachangelog.com/en/1.0.0/ for examples --> ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [conventional commit guidelines](https://www.conventionalcommits.org/en/v1.0.0/) * [x] I ran `just p` (fmt, clippy and test) before committing #### New Features: * [x] I've added tests for the new feature * [ ] I've added docs for the new feature #### Bugfixes: * [ ] This pull request breaks the existing API * [x] I've added tests to reproduce the issue which are now passing * [x] I'm linking the issue being fixed by this PR ACKs for top commit: nymius: ACK 6d61ce3 Tree-SHA512: 01f3398c7f441ae165dc7e7eccd967ba641c20b565c884216b7cadef446704412297e3c659f1783996ae7acca5b9f969dd2951c4eec935200b2cf52605d96bf5
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Description
Fixes #67 by adding const K_MAX: u32 = 2323 to silentpayments::receive, and a threshold check to the matched_tweaks counter loop in scan_txouts. Also added a ScanLimitExceeded(u32) variant to SpReceiveError returning an explicit error
Notes to the reviewers
I replaced
check_cases(27)with a dedicatedmaximum_per_group_recipient_limit_k_max_is_exceeded()test function that explicitly matches and asserts SpReceiveError::ScanLimitExceeded(K_MAX) on test vector 27, preservingcheck_casesfor standard test vectors. i think modifying it with hardcoded branch conditions for test 27 makes the general test runner dirty.All Submissions:
just p(fmt, clippy and test) before committingNew Features:
Bugfixes: