fix(executor): fetch related contracts on update_state requires(missing)#4007
Merged
Conversation
PR #4006 added network-fallback for the validate_state RequestRelated branch but missed the parallel update_state path. When a contract's update_state returns UpdateModification::requires(missing), bridged_upsert_contract_state still mapped Either::Right straight to MissingRelated without attempting local lookup or network fetch. Cross-node UPDATE flows that only need related state at merge time (e.g. inbox.update_state needing the sender's AFT record) bounced through ResyncRequest fallback even when the related contract was reachable via local store or network GET. Surfaced in freenet/mail#80. Mirror the validate_state path: walk each requires(missing) id with local-first / network-second fetch via fetch_related_via_network, append RelatedState entries to the update slice, re-attempt the merge. Depth limit preserved (a second requires after retry rejects). Mock executor gets a new UpdateOverride::RequiresRelated to drive the path; new test_update_state_requires_related_fetches_and_retries pins the happy path with a stubbed network fetcher. contracts.md updated to document both branches.
Contributor
Rule Review: No issues foundRules checked: WarningsNone. Info
Notes on what was verified:
Rule review against |
Three changes addressing the rule-review: 1. update_state requires() path now applies the same abuse-prevention guards as the validate-side path: empty-list rejection, self-reference rejection, MAX_RELATED_CONTRACTS_PER_REQUEST cap (10), id dedup. Without these a misbehaving contract could fan out unbounded network GETs by returning an oversized requires() list. 2. Network-fetch failure now logs warn with related_id + error before mapping to MissingRelated. Previously the error was swallowed by Err(_). 3. Three new edge-case tests: requires+network-fail, depth>1 rejection (always-requires override), and too-many cap. Plus self-reference rejection. Mock gets a new UpdateOverride::AlwaysRequiresRelated to drive the depth and oversize paths.
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
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.
Summary
PR #4006 added network-fallback for the
validate_stateRequestRelatedbranch but missed the parallelupdate_staterequires(missing) path. When a contract'supdate_statereturnsUpdateModification::requires(missing),bridged_upsert_contract_statestill mapped theEither::Right(missing)outcome straight toMissingRelatedwithout attempting local lookup or network fetch.This is the actual root cause behind freenet/mail#80: the inbox contract's
update_statereturnsrequires(AFT-record)on the receiver because it hasn't seen the sender's AFT yet. The mail UI workaround inline-bundles the related state viaRelatedStateAndDeltato dodge the path entirely. After this PR the runtime resolves the related contracts itself (local first, then network viastart_sub_op_getwhenop_manageris wired), and the workaround can be removed.Fix
Mirror the validate-side flow:
Either::Right(missing_related), walk each id with local-first / network-second fetch viafetch_related_via_network(introduced in fix(executor): escalate UPDATE related-fetch to network when op_manager is wired #4006).UpdateData::RelatedStateentries to the update slice.requires, reject (depth-1 limit, matching validate-side behavior).Test plan
MockWasmRuntimegets a newUpdateOverride::RequiresRelatedto drive the pathtest_update_state_requires_related_fetches_and_retriespins the happy path: stub records the network call, second update_state succeeds with RelatedState entries, upsert resolves to Okpool_tests::related_contract_testspass locallycargo clippy --all-targetscleancontracts.mdupdated to document both branchesE2E
E2E validated against the iso-nodes harness with this binary: alice→bob cross-node delivery completes via the runtime's own resolution path (no UI inline-bundling needed). The mail-side
RelatedStateAndDeltaworkaround is now redundant; will follow up with a mail PR removing it once this lands.