Skip to content

fix(di): validate contract changes at the transaction boundary - #580

Merged
wolfy-j merged 2 commits into
mainfrom
fix/contract-transactional-validation
Aug 19, 2026
Merged

fix(di): validate contract changes at the transaction boundary#580
wolfy-j merged 2 commits into
mainfrom
fix/contract-transactional-validation

Conversation

@wolfy-j

@wolfy-j wolfy-j commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

The defect

Adding a method to a live contract definition is impossible. The module upgrade produces one changeset carrying the definition update and its binding updates, but di.Manager validates each entry against already-committed state:

  • definition first (what the topo sort produces): handleDefinitionUpdate re-validates the still-old binding → contract method is not bound
  • binding first: handleBindingUpdate validates against the still-old definition → bound method is not defined in contract definition

Both are apierror.Invalid → fatal to the runner → full rollback, every ordering. The self-healing deferral loop cannot rescue it — it retries only NotFound, and neither half is individually valid against the pre-change peer: the dependency is a genuine two-cycle at per-entry granularity.

Boot never hits this: a fresh load delivers everything as EntryCreate in dependency order, and handleDefinitionAdd does not cross-validate. The rejection is specific to EntryUpdate on a live instance — exactly the hub module-upgrade path. (Downstream, keeper surfaces any apply failure as HTTP 409, which is how this presents to operators.)

Encountered in production: upgrading kickside/contract to add a method to a live contract definition 409s in every ordering — single-module install, co-planned install with version floors, both directions.

The fix

di.Manager now implements registry.TransactionListener — the mechanism the registry has offered since the tx protocol landed (Dec 2024), already used by service/net and the Lua code manager.

  • Begin opens a staging area.
  • During the transaction entry operations validate structurally, stage against an effective view (so same-transaction operations see each other — existence checks included), and queue their contract-plane events. Cross-entry validation defers.
  • Commit validates the complete post-transaction state — every effective binding against every effective definition, plus unique defaults — then applies and flushes events atomically. A validation failure leaves committed state untouched, emits nothing, and returns the error; the runner rolls back and dispatches TxDiscard.
  • Discard drops the staging, and tolerates running after a failed Commit (the runner discards after a commit failure).

Outside a transaction, behaviour is unchanged — per-entry validation, immediate apply, immediate events. All pre-existing tests pass untouched.

The definition-in-use check moves to Commit inside a transaction, where a definition deleted while a binding survives surfaces as binding references undefined contract; retiring a contract together with its bindings in one changeset now also works.

Semantics notes for review

  • Events are queued in staging order and flushed on Commit only, so the contract plane never observes an intermediate invalid state.
  • If another participant fails Commit after this manager committed, the runner's rollback ops arrive outside a transaction and apply immediately — correct restoration; the trailing TxDiscard is a no-op.
  • In-transaction Add no longer needs the runner's NotFound deferral for binding-before-definition ordering; Commit sees both.

Testing

  • New manager_tx_test.go: the live-upgrade case in both orders, commit rejection (state untouched, zero events emitted), discard, delete-together vs delete-alone, in-transaction create order independence, duplicate defaults across staged+committed bindings, staged-state visibility to existence checks.
  • go test ./service/di/ ./system/registry/... ./boot/... ./service/... ./internal/... ./api/... -race -short — all pass.
  • golangci-lint v2.8.0 run --build-tags=race ./service/di/... — 0 issues.

@wolfy-j
wolfy-j requested a review from skhaz August 19, 2026 02:51
Adding a method to a live contract definition is impossible today: the
definition update and its binding updates arrive in one changeset, but the
contract manager validates each entry against already-committed state. The
definition update re-validates the still-old binding ("contract method is not
bound"); applied the other way around, the binding update validates against
the still-old definition ("bound method is not defined in contract
definition"). Both are apierror.Invalid, which the registry runner treats as
fatal, so every ordering rolls the changeset back. The runner's deferral loop
cannot help: it retries only NotFound, and neither half is individually valid
against the pre-change peer regardless of ordering - the dependency is a
genuine two-cycle at per-entry granularity.

Boot never hits this because a fresh load delivers everything as EntryCreate
in dependency order, and handleDefinitionAdd does not cross-validate. The
rejection is specific to EntryUpdate on a live instance, which is exactly the
module-upgrade path.

The registry has offered the fix since the tx protocol landed:
registry.TransactionListener. The manager now participates. Begin opens a
staging area; entry operations validate structurally, stage their mutation
against an effective view (so same-transaction operations see each other),
and queue their contract-plane events. Commit validates every effective
binding against every effective definition plus unique defaults - the
complete post-transaction state - then applies and flushes events atomically.
A commit-time failure leaves committed state untouched and emits nothing; the
runner rolls back and discards. Discard drops the staging, and tolerates
running after a failed Commit.

Outside a transaction, behaviour is byte-for-byte what it was: per-entry
validation, immediate apply, immediate events. The definition-in-use check
and the update-would-invalidate check still guard direct operations; inside a
transaction both defer to Commit, where a definition deleted while a binding
survives surfaces as an unresolved contract reference.

Tests cover the live-upgrade case in both orders, commit rejection with state
and event silence, discard, delete-together vs delete-alone, in-transaction
create order independence, duplicate defaults across staged and committed
bindings, and staged-state visibility to existence checks.

go test ./service/di/ ./system/registry/... ./boot/... ./service/...
./internal/... ./api/... -race -short passes; golangci-lint v2.8.0 reports
0 issues on the package.
@wolfy-j
wolfy-j force-pushed the fix/contract-transactional-validation branch from a39214e to 46c09bb Compare August 19, 2026 13:55
@wolfy-j
wolfy-j force-pushed the fix/contract-transactional-validation branch from 46c09bb to 34548e6 Compare August 19, 2026 13:58
@wolfy-j
wolfy-j merged commit 1041a8f into main Aug 19, 2026
4 checks passed
@wolfy-j
wolfy-j deleted the fix/contract-transactional-validation branch August 19, 2026 14:08
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.

2 participants