Skip to content

fix(provider): add P2TR address matching to key verification#86

Merged
oritwoen merged 3 commits into
mainfrom
fix/provider-p2tr-verify
Mar 22, 2026
Merged

fix(provider): add P2TR address matching to key verification#86
oritwoen merged 3 commits into
mainfrom
fix/provider-p2tr-verify

Conversation

@oritwoen
Copy link
Copy Markdown
Owner

The `verify_key_boha` function checks P2PKH compressed, P2PKH uncompressed, and P2WPKH addresses against puzzle targets, but skips P2TR. If a puzzle uses a Taproot address, `vuke analyze --verify` won't match it even with the correct key.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 15, 2026

📝 Walkthrough

Walkthrough

The verify_key_boha function now matches Taproot-derived addresses: when derived.p2tr equals the puzzle's address, it records a match with address_type set to "p2tr", extending existing support for compressed/uncompressed pubkey-hash and witness variants.

Changes

Cohort / File(s) Summary
Taproot Address Matching
src/provider.rs
Added P2TR case to address-type matching in verify_key_boha, completing address type recognition alongside p2pkh_compressed, p2pkh_uncompressed, and p2wpkh.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Poem

🔑 Taproot joins the match,
where keys find their address home,
one more path to check. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed Title clearly describes the main change: adding P2TR address matching to the verify_key_boha function's key verification logic.
Description check ✅ Passed Description explains the problem (P2TR addresses skipped during verification), the impact (vuke analyze --verify fails), and the solution (adding P2TR matching).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/provider-p2tr-verify
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix/provider-p2tr-verify

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Requires human review: This PR modifies cryptographic key verification logic by adding P2TR address matching, which requires human domain knowledge to ensure the derivation and comparison are correct.

coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 22, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/provider.rs`:
- Around line 442-443: The test coverage misses the P2TR branch: update tests to
assert the address_type is "p2tr" or add a new test that uses a known Taproot
puzzle/address; specifically modify or extend test_verify_key_match (or add a
new test) to provide a puzzle/address that maps to derived.p2tr and then assert
the function returns Some("p2tr") (or that address_type == "p2tr"), ensuring the
p2tr branch in provider.rs (the derived.p2tr == addr check) is executed.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5956631d-8600-4e28-8892-0058167fc73a

📥 Commits

Reviewing files that changed from the base of the PR and between 0789ea2 and 41b50af.

📒 Files selected for processing (1)
  • src/provider.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Order imports as: external crates → std → blank line → super:: → blank line → crate::
Prefer ? operator over .unwrap() in new code
Use PascalCase for types and structs
Use snake_case for function and method names
Use SCREAMING_SNAKE_CASE for constants
Use snake_case for file and module names

Files:

  • src/provider.rs
src/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.rs: Use indicatif::ProgressBar for long-running operations
Place inline tests in #[cfg(test)] mod tests at the end of each file, using standard assert! and assert_eq! macros
Use tempfile crate for file I/O tests

Files:

  • src/provider.rs
🧠 Learnings (1)
📚 Learning: 2026-03-13T13:20:58.369Z
Learnt from: oritwoen
Repo: oritwoen/vuke PR: 76
File: src/source/wordlist.rs:134-143
Timestamp: 2026-03-13T13:20:58.369Z
Learning: Do not flag .ok() calls on output.hit() / output.key() inside Rayon par_chunks().for_each() closures in Rust files (e.g., src/source/wordlist.rs) as bugs. Rayon for_each requires FnMut() -> (), which cannot propagate Result with ?. This is a known limitation discussed in issue `#77`. Treat these .ok() usages in this pattern as intentional and not regressions; focus review on actual logic or error handling outside this specific closure pattern.

Applied to files:

  • src/provider.rs

Comment thread src/provider.rs
@oritwoen oritwoen self-assigned this Mar 22, 2026
@oritwoen oritwoen merged commit decd22e into main Mar 22, 2026
2 checks passed
@oritwoen oritwoen deleted the fix/provider-p2tr-verify branch March 22, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant