refactor(contract): move shared request and auth plumbing into api/common.rs

[gilcu3]

Part 2/12 of #1962

[claude]

Pull request overview

Part 2/12 of the #1962 contract-refactor series. It carves the request/auth plumbing that had accumulated in crates/contract/src/lib.rs out into a new crates/contract/src/api/common.rs, alongside the tests that cover it. Comparing the removed and added blocks line by line, this is a faithful relocation: no control flow, error string, gas/deposit constant, or test assertion changed — only item visibilities (fn/const to pub(crate)), two doc-comment touch-ups, and the module boundary.

Changes:

• New api/common.rs holding: check_request_preconditions, enqueue_yield_request, voter_account, caller_is_signer, voter_or_panic, assert_caller_is_attested_participant_and_protocol_active, assert_caller_is_signer, the #[private] fail_on_timeout entrypoint, require_deposit, refund_to, DATA_ID_REGISTER, and MINIMUM_SIGN_REQUEST_DEPOSIT.
• The six check_request_preconditions__* unit tests and their override_context_for_preconditions helper move with the code.
• lib.rs imports the three now-cross-module items (MINIMUM_SIGN_REQUEST_DEPOSIT, refund_to, require_deposit) and drops the newly unused RequestError / GasWeight imports.
• Doc touch-ups: MINIMUM_SIGN_REQUEST_DEPOSIT now records that it is also charged for foreign-tx verification (matches lib.rs:420), and the ProtocolContractState intra-doc links are re-qualified as crate::state::... now that the type is no longer in scope.

Reviewed changes

Per-file summary

File	Description
crates/contract/src/api.rs	Declares the new pub mod common;
crates/contract/src/api/common.rs	New module: request preconditions, yield-promise plumbing, deposit/refund helpers, signer/voter/attestation assertions, fail_on_timeout, plus the relocated unit tests
crates/contract/src/lib.rs	Removes the relocated items and their tests; adjusts imports

Findings

No blocking issues. Things I checked that hold up:

• fail_on_timeout is dispatched by the method_names::FAIL_ON_TIMEOUT string constant (lib.rs:2341, 2480, 2518), so moving it between #[near] impl blocks does not affect the call sites; the ABI snapshot lists functions alphabetically and this entry carries no doc field, so no snapshot churn is expected.
• assert_matches is a regular (non-dev) dependency (crates/contract/Cargo.toml:77), so assert_caller_is_attested_participant_and_protocol_active still compiles outside cfg(test).
• The design-doc references to enqueue_yield_request / assert_caller_is_signer in docs/design/attestation-verifier-contract.md are commit-pinned permalinks, so they do not rot from this move.
• require_deposit, refund_to, TeeError, and InvalidParameters all still have live users in lib.rs; the test-module glob (use super::*) picks up the parent dtos alias as before.

Non-blocking (nits, follow-ups, suggestions):

• crates/contract/src/api.rs:1 — the module doc still reads "one module per feature", but common is explicitly not a feature (common.rs:1: "Plumbing shared by more than one feature"). Worth a one-line amendment now that the first non-feature module lands, e.g. "one module per feature, plus common for shared plumbing".
• crates/contract/src/api/common.rs:27,52,64,68 — the // 1. … // 4. step comments restate the numbered list already in the doc comment at lines 14-17, and each paraphrases the call directly beneath it (// 3. Require the minimum deposit and refund any excess. sits on top of require_deposit(minimum_deposit, &predecessor)). Per docs/engineering-standards.md (Write helpful code comments), step-labelling comments are the kind to strip, and relocating the function is a cheap moment to do it. Pre-existing, so optional here.
• crates/contract/src/api/common.rs:207 (MINIMUM_SIGN_REQUEST_DEPOSIT, moved) vs crates/contract/src/lib.rs:105 (MINIMUM_CKD_REQUEST_DEPOSIT, left behind) — the two sibling deposit constants now live in different modules. Justifiable while only the sign constant is shared by two future feature modules, but a later part of the series should land MINIMUM_CKD_REQUEST_DEPOSIT next to it (or in the CKD module) rather than leaving the split permanent.
• crates/contract/src/api/common.rs:267,284,297,310,323,336 — the relocated tests predate the <system_under_test>__should_<assertion> convention in CLAUDE.md. Not a merge blocker for a move-only PR, but an opportunistic rename (check_request_preconditions__should_panic_when_domain_does_not_exist, etc.) would bring them in line while they are already being touched.
• crates/contract/src/api/common.rs — with request preconditions, yield plumbing, deposit/refund, and caller authentication all in one 347-line common, this module is the natural place for later parts of refactor: Break out feature-specific contract methods and tests to their own modules #1962 to deposit anything that does not fit elsewhere. If the plan does not already cover it, consider splitting along the seam the PR title itself names — request plumbing vs. auth — before it grows.

✅ Approved

[pbeza]

I won't lie, Claude helped me confirm this is a mechanical refactor. Only these changes aren't mechanical (caught by Claude), but they look good:

• Visibility widened from private to pub(crate) on the moved items, which the move requires and which changes nothing externally
• Doc-comment-only edits: intra-doc links rewritten to full paths (crate::state::ProtocolContractState::...) so they resolve from the new module, and one added clause on MINIMUM_SIGN_REQUEST_DEPOSIT ("also charged for foreign-transaction verification")
• Import shuffling in lib.rs (drops now-unused GasWeight and RequestError, imports the moved items from api::common) and the new file's own imports, module doc line, impl-block/test-module scaffolding, and closing braces

[kevindeforth]

I would suggest to split this into even more files.

[kevindeforth]

Hmm, I think it wouldn't be bad to have maybe one file where we track all the promises that this contract spawns (e.g. promises.rs).

This method and enqueue_yield_request kind of belong together in that sense.

[gilcu3]

Unfortunately we have many more methods using promises than this two, so it would not be very uniform to have them being located there. Leaving here for now, we could think later if we want to do some promise unification

[kevindeforth]

I pre-approved, because it's an improvement and I don't want to block, but I would still suggest to consider splitting this further.