chore!: clear deployment state and flag the fork as unaudited - #46
Merged
Conversation
Nothing in this tree is deployed, and the artefacts still said otherwise. - removes the legacy root `networks.json`. It recorded upstream's addresses for contracts this fork no longer matches, so leaving it invited those addresses to be read as the fork's own. When this fork does deploy, the canonical record is `deployments/networks.json` - `dev/verify-contracts.sh` now fails with a clear message instead of feeding `null` addresses into `forge verify-contract` - drops the README deployment address table and states plainly that there are no deployments - reframes the audit list: those audits cover upstream before this work, and do not carry over - adds a warning under the README title that the fork is unaudited, not deployed, and substantially changed BREAKING CHANGE: `networks.json` is removed. Consumers resolving addresses from it were resolving upstream deployments, not this fork's.
mfw78
added a commit
that referenced
this pull request
Aug 1, 2026
Closes #5. An empty manifest page already carried `reasonCode`, so the issue is not entirely as written. But `reasonCode` is the selector of an error the *handler* declares (`TooEarly.selector`, `OrderNotInitialized.selector`), so a consumer that has not been taught a particular handler's error names still cannot tell a not-yet-active order from a permanently invalid one. Both render as "no orders". The verdict and the wait timestamp were both decoded by `_decodeErrorToGeneratorResult` and then discarded on the way out. ## The change `getManifestPage` returns a `ManifestStatus` in place of the bare selector: ```solidity struct ManifestStatus { IConditionalOrderGenerator.GeneratorResultCode code; uint256 waitUntil; bytes4 reasonCode; } ``` `code` is the same verdict `poll` would return for the same conditions, decoded through the same `_decodeErrorToGeneratorResult`. The manifest mirrors the poll path rather than becoming a second source of truth for whether an order is live. `reasonCode` is unchanged and remains the handler-declared detail, useful once a consumer does know the handler. An ordinary page reports `POST` with zeroes, including a page that is empty only because `offset` is past the end. That is what separates "nothing here" from "nothing yet": both return no entries, and only `code` distinguishes them. ## All three implementations move together Two helpers on `BaseConditionalOrder`, `_manifestOk()` and `_manifestStatus(errorData)`, so the error-to-status mapping is stated once. `TWAP` and `PerpetualStableSwap` call them rather than restating it, which is what keeps the three from drifting apart later. `TWAP`'s uninitialised-context page now reports `TRY_NEXT_BLOCK` rather than an unqualified empty page. A context that has not been written yet is transient, and reporting it as an ordinary empty page is the same conflation the issue is about. ## Breaking `getManifestPage`'s third return value changes type. #46 established there are no deployments, so no consumer is affected; that is the reason for doing this now rather than adding a parallel accessor. ## Verification 166 tests pass, `forge fmt --check` clean, `forge build` clean. The two assertions that carry this change were checked against an implementation that decodes the verdict and then discards it, keeping only `reasonCode` (the old behaviour): ``` mutation applied: verdict discarded, reasonCode kept test_manifestPage_EmptyPageCarriesWaitReason: FAIL test_manifestPage_EmptyPageCarriesInvalidReason: FAIL restored: 166 tests passed ``` Both fail without the fix, so they are testing the behaviour rather than passing either way. ## How to test ```sh export FOUNDRY_PROFILE=ci forge fmt --check && forge build && forge test --no-match-test fork --fuzz-seed 672679878 ```
This was referenced Aug 1, 2026
mfw78
added a commit
that referenced
this pull request
Aug 1, 2026
Part of #35, first bullet. Does not close it; see below. #41 renamed the polling agent to "monitoring service" across `docs/`, but README.md was never touched. The user-facing document was still the one describing the term the rename was meant to retire. ## What changed **README.md**, five references. Four are the generic polling-agent sense and become "off-chain monitoring service" or "monitoring services", matching the terminology #41 established. The fifth is the link to [`cowprotocol/tenderly-watch-tower`](https://github.com/cowprotocol/tenderly-watch-tower) in the local-deployment section. Dropped rather than relabelled: it points at upstream's specific implementation, which is not the monitoring service this repository describes, and nothing in the surrounding `anvil` instructions depends on that tool in particular. The sentence now refers generically to running a monitoring service against the deployment. **`docs/architecture.md` and `docs/discovery.md`**, the two glossary parentheticals. `architecture.md` dropped "(historically called watch-towers)", and `discovery.md` dropped "(the polling agent historically called a watch-tower; "monitoring service" hereafter)". These were bridges for readers arriving from upstream, written while the rename was in flight. This is a judgement call and the easiest thing here to disagree with. The argument for keeping them is that upstream's term is what someone will have in their head when they arrive. The argument against, which I went with: the documents that retire the term are the wrong place to keep it alive, and a reader who knows the upstream term will map it from context in one sentence. `discovery.md` needed a small reword. Removing the parenthetical left "let an off-chain monitoring service service handlers", so that is now "support handlers". **`test/ComposableCow.twap.t.sol`**, one stale comment, no behaviour. ## What is deliberately left `broadcast/StandardJsonInput/*.json` still contain the term, six files. Those are verbatim flattened-source verification inputs rather than prose, so rewriting the term inside them would corrupt what they are for. They are stale independently of this: they carry pre-typed-errors signatures such as `PollTryNextBlock(string reason)`, and one is still named `ComposableCoW.json` in the old casing. Given #46 established there are no deployments, the whole of `broadcast/` is arguably dead weight, but that is a separate call from a terminology fix, and deleting deployment history deserves its own PR. ## Why #35 stays open Its third bullet asks for the canonical deployments manifest (chainId to registry address, deploy block, ABI version, topic0 set). `deployments/networks.json` is `"networks": {}` after #46, and there is nothing to publish until something is deployed. The verdict-permanence bullet was covered by #41. ## Verification Docs and one comment only, no source change. 166 tests pass, `forge fmt --check` clean, build clean. `rg -ni 'watch.?tower|tenderly'` returns nothing outside `broadcast/`.
mfw78
added a commit
that referenced
this pull request
Aug 1, 2026
The warning added in #46 says the fork has diverged substantially and lists the surfaces in one sentence, which is enough to stop someone deploying it but not enough to tell them what is actually different. A reader arriving from cowprotocol/composable-cow had to reconstruct that from the commit log. Adds a "Divergence from upstream" section, one line per change, covering order generation, typed errors, structured polling, the fill overlay, the order manifest, handler discovery, proof payload locations, handler validation and housekeeping. The warning now points at it. Also records that upstream has since added ComposableCowPoller and that this tree does not have it. The divergence runs both ways and a section that only lists what was added here would read as a superset. Two parts of the README described interfaces that no longer exist, which is what prompted checking the rest of it: - The merkle root instructions still described `Proof` as a `location` of 0 or 1 with an opaque payload. It has been `{uris, blobVersionedHashes}` since #39. - `getTradeableOrderWithSignature` was described as reverting with custom errors for order conditions. Since #23 it returns a structured `PollResult` and reverts only for authorisation and handler-interface failures, so the documented integration was the one the change replaced. Every claim in the new section was checked against the tree rather than the commit subjects: the settlement entry point is signature-identical to upstream, which is what "shape-compatible" rests on.
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
1 participant
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.
Nothing in this tree is deployed, and the artefacts still said otherwise.
networks.json(274 lines, 8 contracts x 8 chains). Those are upstream's addresses, for contracts this fork no longer matches after the generateOrder rename, typed errors, poll interface and proof-payload changes. Leaving the file invited them to be read as this fork's own deployments. When this fork does deploy, the canonical record isdeployments/networks.json, landing in docs: monitoring-service terminology, verdict permanence, deployments manifest聽#41.dev/verify-contracts.shwas the file's only consumer. It now fails with a clear message instead of feedingnulladdresses intoforge verify-contract.BREAKING:
networks.jsonis removed. Any consumer resolving addresses from it was resolving upstream deployments, not this fork's.How to test
Part of #1.