fix(drive): consolidate historical contract proof verification retry logic

[thepastaclaw]

Issue

When getDataContract is called for a contract with keepsHistory: true, it returns "Data contract not found" even though getDataContractHistory succeeds for the same contract.

Root cause: In verify_contract_v0, when contract_known_keeps_history is None (as it always is from FromProof<GetDataContractRequest>), the code defaults to the non-historical path. If that returns Ok(None) (contract not found on the non-historical path), it never retries with the historical path — it only retried on Err(_).

Changes

Core fix (both verify_contract and verify_contract_return_serialization)

• Extracted _given_history helpers that perform verification with a known history flag
• Consolidated retry logic into the entry point: when contract_known_keeps_history is None, retry with keeps_history = true if the first attempt returns Err(_) or Ok((_, None))
• Removed scattered retry logic from deep inside the function body

Tests

• Added test_contract_keeps_history_verify_with_unknown_history_flag regression test covering:
  1. None (unknown) — must succeed via retry for historical contracts
  2. Some(true) — direct historical verification
  3. Some(false) — non-historical contract verification
  4. verify_contract_return_serialization with None — parallel path coverage

Validation

• cargo test -p drive ✅
• cargo clippy -p drive -- -D warnings ✅

Summary by CodeRabbit

• Bug Fixes

  • Improved contract verification logic to better handle cases where contract history state is unknown, with automatic retry capability when needed.

• Tests

  • Added regression tests validating contract verification behavior across different history state scenarios.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

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 `@packages/rs-drive/tests/query_tests.rs`:
- Around line 4702-4703: Rename the test function
test_contract_keeps_history_verify_with_unknown_history_flag to follow the
repository convention by starting with "should_"; update the function signature
(fn test_contract_keeps_history_verify_with_unknown_history_flag) to a
descriptive name beginning with should_, e.g.
should_contract_keep_history_verify_with_unknown_history_flag, and update any
references or test runners expecting the old name so the test still compiles and
runs.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78184f6 and f992ab4.

📒 Files selected for processing (3)

• packages/rs-drive/src/verify/contract/verify_contract/v0/mod.rs
• packages/rs-drive/src/verify/contract/verify_contract_return_serialization/v0/mod.rs
• packages/rs-drive/tests/query_tests.rs

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Rename the new test to follow the repository test naming rule.

The new test name should start with should_... to match the project’s test naming convention.

✏️ Suggested rename

- fn test_contract_keeps_history_verify_with_unknown_history_flag() {
+ fn should_verify_contract_keeps_history_when_history_flag_is_unknown() {

As per coding guidelines: **/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should …'.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	#[test]
	fn test_contract_keeps_history_verify_with_unknown_history_flag() {
	#[test]
	fn should_verify_contract_keeps_history_when_history_flag_is_unknown() {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/rs-drive/tests/query_tests.rs` around lines 4702 - 4703, Rename the
test function test_contract_keeps_history_verify_with_unknown_history_flag to
follow the repository convention by starting with "should_"; update the function
signature (fn test_contract_keeps_history_verify_with_unknown_history_flag) to a
descriptive name beginning with should_, e.g.
should_contract_keep_history_verify_with_unknown_history_flag, and update any
references or test runners expecting the old name so the test still compiles and
runs.