SIG-625: add fork test for operator upgrade lifecycle#70
Conversation
Verify on prod fork: - TradeModule upgrade via setModules preserves vault state - Router lifecycle (increase/decrease/close/claim) emits actual user as trader - No spurious NFT Transfer events to/from Router - Non-owner caller rejected by Router owner guard
![signals-reviewer[bot]](https://avatars.githubusercontent.com/u/206232752?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
End-to-end fork test for SIG-625's operator upgrade lifecycle. Upgrades TradeModule on a production fork, deploys a new SignalsRouter, and verifies the full position lifecycle (increase, decrease, close, claim) works correctly with ERC721 approved operators — confirming that trade events reference the actual trader (not the router) and no spurious NFT transfers occur.
Cross-PR Context
Sibling PRs (SIG-625)
v1-contractPR #69 (MERGED): SIG-625: accept ERC721 approved operators for position lifecycle auth- Impact map check: This PR is test-only — no ABI, event, or interface changes. No downstream repo updates required.
Issues
No issues found.
Suggestions
No suggestions.
Verdict
APPROVE: Test-only PR with thorough coverage of the operator upgrade lifecycle. Event signature constants match the actual contract definitions. Settlement flow timing (submitWindow, claimDelay) is correct. The _findActiveMarket search, tick alignment, and log assertion helpers are all well-implemented.
There was a problem hiding this comment.
Summary
End-to-end fork test for SIG-625's operator upgrade lifecycle. Upgrades TradeModule on a production fork, deploys a new SignalsRouter, and verifies the full position lifecycle (increase, decrease, close, claim) works correctly with ERC721 approved operators — confirming that trade events reference the actual trader (not the router) and no spurious NFT transfers occur.
Cross-PR Context
Sibling PRs (SIG-625)
v1-contractPR #69 (MERGED): SIG-625: accept ERC721 approved operators for position lifecycle auth- Impact map check: This PR is test-only — no ABI, event, or interface changes. No downstream repo updates required.
Issues
No issues found.
Suggestions
No suggestions.
Verdict
APPROVE: Test-only PR with thorough coverage of the operator upgrade lifecycle. Event signature constants match the actual contract definitions. Settlement flow timing (submitWindow, claimDelay) is correct. The _findActiveMarket search, tick alignment, and log assertion helpers are all well-implemented.
There was a problem hiding this comment.
Summary
End-to-end fork test for SIG-625's operator upgrade lifecycle. Upgrades TradeModule on a production fork, deploys a new SignalsRouter, and verifies the full position lifecycle (increase, decrease, close, claim) works correctly with ERC721 approved operators — confirming that trade events reference the actual trader (not the router) and no spurious NFT transfers occur.
Cross-PR Context
Sibling PRs (SIG-625)
v1-contractPR #69 (MERGED): SIG-625: accept ERC721 approved operators for position lifecycle auth- Impact map check: This PR is test-only — no ABI, event, or interface changes. No downstream repo updates required.
Issues
No issues found.
Suggestions
No suggestions.
Verdict
APPROVE: Test-only PR with thorough coverage of the operator upgrade lifecycle. Event signature constants match the actual contract definitions. Settlement flow timing (submitWindow, claimDelay) is correct. The _findActiveMarket search, tick alignment, and log assertion helpers are all well-implemented.
1 participant
Context
Fork test for the operator upgrade lifecycle introduced in SIG-625 (PR #69).
Validates that after upgrading TradeModule and deploying a new SignalsRouter on a production fork, the approved-operator pattern works end-to-end with real on-chain state.
Decisions
ForkBaseTestbase class for prod fork infrastructure (RPC, contract addresses, owner safe)claimPayouttest usesmarkSettlementFailed→finalizeSecondarySettlementpath (deterministic tick placement without oracle dependency)if (address(router) == address(0)) return) gracefully skips when fork env lacks required contract addressesSibling PRs
v1-contractPR SIG-625: accept ERC721 approved operators for position lifecycle auth #69: SIG-625: accept ERC721 approved operators for position lifecycle auth (CLOSED)