fix(p0-2,p0-3): CEI order in PaymasterFactory + Registry unchecked call checks

[jhfnetboy]

Summary

Two P2 hygiene fixes from the Slither/Opus audit review (2026-06-28). Neither is a GA blocker (both guarded by nonReentrant / existing checks), but resolves the Slither findings cleanly.

P0-2 — PaymasterFactory CEI ordering (PaymasterFactory.sol)

• deployPaymaster: state writes (paymasterByOperator, operatorByPaymaster, paymasterList, totalDeployed) now happen before _initAndVerify external calls
• deployPaymasterDeterministic: same fix applied
• nonReentrant already prevents re-entry; this brings the code into textbook CEI alignment

P0-3 — Registry unchecked low-level call (Registry.sol)

• _initRole: (bool ok,) = address(GTOKEN_STAKING).call(...) — return value captured; emit SyncFailed on failure
• _syncExitFeeForRole: same pattern — now consistent with the existing public syncExitFees() implementation

Test plan

• forge test — 1080/1080 pass, 0 failed
• No Sepolia deploy needed (no interface change, no storage layout change)
• Include in next scheduled release

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[clestons]

APPROVE — fixes are correct; one Medium design concern noted.

P0-2: PaymasterFactory CEI (PaymasterFactory.sol)

State writes (paymasterByOperator, operatorByPaymaster, paymasterList, totalDeployed) now precede _initAndVerify external call in both deployPaymaster and deployPaymasterDeterministic. CEI pattern is correct. The existing nonReentrant modifier already blocks re-entry; the CEI reorder adds hygiene alignment and future-proofs against any removal of nonReentrant. 1080 forge tests pass. ✓

P0-3: Registry unchecked low-level call (Registry.sol)

(bool ok,) = address(GTOKEN_STAKING).call(...) + if (!ok) emit SyncFailed(...) correctly captures the return value. This is consistent with the public syncExitFees() pattern (soft failure). ✓

[Medium — design] P0-3 soft failure leaves Registry/GTokenStaking state inconsistent

When _initRole writes roleConfigs[roleId] = RoleConfig(..., isActive: true, ...) and then the GTokenStaking sync call fails, the role is permanently active in Registry with no corresponding exit-fee entry in GTokenStaking. The Slither report recommended require(ok, string(err)) (hard revert) to guarantee atomicity. The chosen soft-failure approach is intentionally tolerant — syncExitFees() can be called after recovery — but it means the window between role creation and a successful sync leaves exit-fee enforcement undefined in GTokenStaking (may default to 0, letting operators exit penalty-free until synced).

Recommend documenting the recovery path (syncExitFees() after GTokenStaking availability) or at minimum logging the failed roleId prominently so operators don't activate roles while staking is desynced.

Invariant / E2E: 5/5 invariants pass at 128k calls each; 4/4 E2E pass on Sepolia v5.4.1-rc.1. ✓

PK: F1 (re-entry through duplicate-operator path) challenged — blocked by nonReentrant. F2 (soft-failure state inconsistency) confirmed — Medium design concern, not a GA blocker.

PK Summary | 1 round · 1 challenged (F1 rejected) · 1 confirmed (F2 Medium)