fix(scripts): derive TWAP calldata with t0=now-60 (COW-1077) - #50
Closed
brunota20 wants to merge 1 commit into
Closed
Conversation
The previous `e2e-onchain.sh` pinned a 516-byte hex blob with `t0 = 0` in the ComposableCoW.create() static-input tuple. TWAP handler's `validateData` does NOT reject `t0 = 0` (it only checks `t0 >= type(uint32).max`), so the `create()` tx succeeded but `TWAPOrderMathLib.calculateValidTo` then computed `part = (block.timestamp - 0) / t = ~3.3M`, which is far above the configured `n = 2` and triggers `AFTER_TWAP_FINISHED` on every `getTradeableOrderWithSignature` poll. Surfaced in the COW-1064 dry run (2026-06-18 report §6.4): supervisor logged the `0xc8fc2725...after twap finished` revert per block. Fix: - New `scripts/_twap_calldata.py` encodes the calldata fresh on every invocation with `t0 = int(time.time()) - 60` (backdated 60s so part 0 is Ready as soon as the order is on-chain). Module docstring explicitly warns against re-hardcoding t0. - `scripts/e2e-onchain.sh` Action 1 now shells out to the helper rather than carrying the hex inline. Validates the output is hex-shaped before passing to `cast send`. - `docs/operations/e2e-cow-1064-prep.md` section 2.3 step 3 replaces the pinned blob with a `python3 scripts/_twap_calldata.py` recipe and a historical note pointing at COW-1077. - `docs/operations/e2e-cow-1064-prep.md` section 4.2 recipe gets `import time` + `int(time.time()) - 60` for `t0` so the re-derivation flow does not re-introduce the bug. - `scripts/README.md` Action 1 description updated to mention the helper. Constants in the helper (sell/buy tokens, amounts, n, t, salt) mirror the prep doc's section 4.2; both must change in lockstep if the TWAP shape is retargeted. Validation: `python3 scripts/_twap_calldata.py` produces 516-byte calldata (1034 hex chars) starting with the correct selector `0x6bfae1ca`; the t0 word reflects current epoch (verified against `0x00...006a3537b5` on the smoke run). `bash -n scripts/e2e-onchain.sh` passes. No engine-side changes; this is a script-and-docs PR. AI assistance disclosure: drafted by Claude (Opus 4.7, 1M context). The original `t0 = 0` calldata was itself drafted by an earlier AI session; this PR returns the encoding to a hand-validated form.
This was referenced Jun 19, 2026
Author
|
Work landed via |
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
mfw78 review of PR #8 (nullislabs#8) flagged "we already pull alloy, so pulling hex via there is really not much of a deal". The PR #47 (COW-1074) commit acc9654 then introduced two new custom hex helpers that recreate the same antipattern at a different scope: - `crates/shepherd-sdk/src/cow/app_data.rs::encode_hex` - 32-byte hash → `0x...`. Used by `resolve_app_data` to format the orderbook lookup path. - `modules/twap-monitor/src/strategy.rs::hex_short` - 8-byte prefix → `0x...…`. Used to format `appData` hashes in INFO log lines. Both crates already depend on `alloy-primitives` (sdk: 1.6, twap-monitor: 1.5), so the swap is a one-liner per call site: - `encode_hex(b)` → `format!("0x{}", alloy_primitives::hex::encode(b))` - `hex_short(b)` → `format!("0x{}…", alloy_primitives::hex::encode(&b[..8]))` Both functions keep their old signature so callers (`resolve_app_data` in the SDK, every `host.log` line in twap-monitor strategy) need no changes. Comments on both helpers now explicitly reference mfw78's PR #8 guidance so the next person tempted to hand-roll a `0123456789abcdef` table has a hook. Validation: cargo test -p shepherd-sdk -p twap-monitor: 32 + 23 passed; cargo clippy --all-targets -- -D warnings: clean; cargo fmt --check: clean; zero em-dash drift. Why this PR sits in a separate branch rather than amending PR #47: PR #47 is already In Review, and #48/#49/#50 stack on top of it. Amending would require force-pushing 4 branches. A small follow-up PR keeps each one bisectable and lets mfw78 review the alloy alignment in isolation.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
mfw78 review of PR #8 (nullislabs#8) flagged "we already pull alloy, so pulling hex via there is really not much of a deal". The PR #47 (COW-1074) commit acc9654 then introduced two new custom hex helpers that recreate the same antipattern at a different scope: - `crates/shepherd-sdk/src/cow/app_data.rs::encode_hex` - 32-byte hash → `0x...`. Used by `resolve_app_data` to format the orderbook lookup path. - `modules/twap-monitor/src/strategy.rs::hex_short` - 8-byte prefix → `0x...…`. Used to format `appData` hashes in INFO log lines. Both crates already depend on `alloy-primitives` (sdk: 1.6, twap-monitor: 1.5), so the swap is a one-liner per call site: - `encode_hex(b)` → `format!("0x{}", alloy_primitives::hex::encode(b))` - `hex_short(b)` → `format!("0x{}…", alloy_primitives::hex::encode(&b[..8]))` Both functions keep their old signature so callers (`resolve_app_data` in the SDK, every `host.log` line in twap-monitor strategy) need no changes. Comments on both helpers now explicitly reference mfw78's PR #8 guidance so the next person tempted to hand-roll a `0123456789abcdef` table has a hook. Validation: cargo test -p shepherd-sdk -p twap-monitor: 32 + 23 passed; cargo clippy --all-targets -- -D warnings: clean; cargo fmt --check: clean; zero em-dash drift. Why this PR sits in a separate branch rather than amending PR #47: PR #47 is already In Review, and #48/#49/#50 stack on top of it. Amending would require force-pushing 4 branches. A small follow-up PR keeps each one bisectable and lets mfw78 review the alloy alignment in isolation.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
mfw78 review of PR #8 (nullislabs#8) flagged "we already pull alloy, so pulling hex via there is really not much of a deal". The PR #47 (COW-1074) commit acc9654 then introduced two new custom hex helpers that recreate the same antipattern at a different scope: - `crates/shepherd-sdk/src/cow/app_data.rs::encode_hex` - 32-byte hash → `0x...`. Used by `resolve_app_data` to format the orderbook lookup path. - `modules/twap-monitor/src/strategy.rs::hex_short` - 8-byte prefix → `0x...…`. Used to format `appData` hashes in INFO log lines. Both crates already depend on `alloy-primitives` (sdk: 1.6, twap-monitor: 1.5), so the swap is a one-liner per call site: - `encode_hex(b)` → `format!("0x{}", alloy_primitives::hex::encode(b))` - `hex_short(b)` → `format!("0x{}…", alloy_primitives::hex::encode(&b[..8]))` Both functions keep their old signature so callers (`resolve_app_data` in the SDK, every `host.log` line in twap-monitor strategy) need no changes. Comments on both helpers now explicitly reference mfw78's PR #8 guidance so the next person tempted to hand-roll a `0123456789abcdef` table has a hook. Validation: cargo test -p shepherd-sdk -p twap-monitor: 32 + 23 passed; cargo clippy --all-targets -- -D warnings: clean; cargo fmt --check: clean; zero em-dash drift. Why this PR sits in a separate branch rather than amending PR #47: PR #47 is already In Review, and #48/#49/#50 stack on top of it. Amending would require force-pushing 4 branches. A small follow-up PR keeps each one bisectable and lets mfw78 review the alloy alignment in isolation.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
mfw78 review of PR #8 (nullislabs#8) flagged "we already pull alloy, so pulling hex via there is really not much of a deal". The PR #47 (COW-1074) commit acc9654 then introduced two new custom hex helpers that recreate the same antipattern at a different scope: - `crates/shepherd-sdk/src/cow/app_data.rs::encode_hex` - 32-byte hash → `0x...`. Used by `resolve_app_data` to format the orderbook lookup path. - `modules/twap-monitor/src/strategy.rs::hex_short` - 8-byte prefix → `0x...…`. Used to format `appData` hashes in INFO log lines. Both crates already depend on `alloy-primitives` (sdk: 1.6, twap-monitor: 1.5), so the swap is a one-liner per call site: - `encode_hex(b)` → `format!("0x{}", alloy_primitives::hex::encode(b))` - `hex_short(b)` → `format!("0x{}…", alloy_primitives::hex::encode(&b[..8]))` Both functions keep their old signature so callers (`resolve_app_data` in the SDK, every `host.log` line in twap-monitor strategy) need no changes. Comments on both helpers now explicitly reference mfw78's PR #8 guidance so the next person tempted to hand-roll a `0123456789abcdef` table has a hook. Validation: cargo test -p shepherd-sdk -p twap-monitor: 32 + 23 passed; cargo clippy --all-targets -- -D warnings: clean; cargo fmt --check: clean; zero em-dash drift. Why this PR sits in a separate branch rather than amending PR #47: PR #47 is already In Review, and #48/#49/#50 stack on top of it. Amending would require force-pushing 4 branches. A small follow-up PR keeps each one bisectable and lets mfw78 review the alloy alignment in isolation.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
mfw78 review of PR #8 (nullislabs#8) flagged "we already pull alloy, so pulling hex via there is really not much of a deal". The PR #47 (COW-1074) commit acc9654 then introduced two new custom hex helpers that recreate the same antipattern at a different scope: - `crates/shepherd-sdk/src/cow/app_data.rs::encode_hex` - 32-byte hash → `0x...`. Used by `resolve_app_data` to format the orderbook lookup path. - `modules/twap-monitor/src/strategy.rs::hex_short` - 8-byte prefix → `0x...…`. Used to format `appData` hashes in INFO log lines. Both crates already depend on `alloy-primitives` (sdk: 1.6, twap-monitor: 1.5), so the swap is a one-liner per call site: - `encode_hex(b)` → `format!("0x{}", alloy_primitives::hex::encode(b))` - `hex_short(b)` → `format!("0x{}…", alloy_primitives::hex::encode(&b[..8]))` Both functions keep their old signature so callers (`resolve_app_data` in the SDK, every `host.log` line in twap-monitor strategy) need no changes. Comments on both helpers now explicitly reference mfw78's PR #8 guidance so the next person tempted to hand-roll a `0123456789abcdef` table has a hook. Validation: cargo test -p shepherd-sdk -p twap-monitor: 32 + 23 passed; cargo clippy --all-targets -- -D warnings: clean; cargo fmt --check: clean; zero em-dash drift. Why this PR sits in a separate branch rather than amending PR #47: PR #47 is already In Review, and #48/#49/#50 stack on top of it. Amending would require force-pushing 4 branches. A small follow-up PR keeps each one bisectable and lets mfw78 review the alloy alignment in isolation.
brunota20
added a commit
that referenced
this pull request
Jun 25, 2026
mfw78 review of PR #8 (nullislabs#8) flagged "we already pull alloy, so pulling hex via there is really not much of a deal". The PR #47 (COW-1074) commit acc9654 then introduced two new custom hex helpers that recreate the same antipattern at a different scope: - `crates/shepherd-sdk/src/cow/app_data.rs::encode_hex` - 32-byte hash → `0x...`. Used by `resolve_app_data` to format the orderbook lookup path. - `modules/twap-monitor/src/strategy.rs::hex_short` - 8-byte prefix → `0x...…`. Used to format `appData` hashes in INFO log lines. Both crates already depend on `alloy-primitives` (sdk: 1.6, twap-monitor: 1.5), so the swap is a one-liner per call site: - `encode_hex(b)` → `format!("0x{}", alloy_primitives::hex::encode(b))` - `hex_short(b)` → `format!("0x{}…", alloy_primitives::hex::encode(&b[..8]))` Both functions keep their old signature so callers (`resolve_app_data` in the SDK, every `host.log` line in twap-monitor strategy) need no changes. Comments on both helpers now explicitly reference mfw78's PR #8 guidance so the next person tempted to hand-roll a `0123456789abcdef` table has a hook. Validation: cargo test -p shepherd-sdk -p twap-monitor: 32 + 23 passed; cargo clippy --all-targets -- -D warnings: clean; cargo fmt --check: clean; zero em-dash drift. Why this PR sits in a separate branch rather than amending PR #47: PR #47 is already In Review, and #48/#49/#50 stack on top of it. Amending would require force-pushing 4 branches. A small follow-up PR keeps each one bisectable and lets mfw78 review the alloy alignment in isolation.
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.
Summary
scripts/e2e-onchain.shpinned a 516-byte hex blob witht0 = 0in the ComposableCoW.create() static-input tuple. TWAP handler'svalidateDatadoes NOT rejectt0 = 0(only checkst0 >= type(uint32).max), so thecreate()tx succeeded - butTWAPOrderMathLib.calculateValidTothen computespart = (block.timestamp - 0) / t = ~3.3M, far above the configuredn = 2, triggeringAFTER_TWAP_FINISHEDon everygetTradeableOrderWithSignaturepoll. The order was dead at submission.Surfaced in the COW-1064 dry-run (2026-06-18 report §6.4): supervisor logged
0xc8fc2725 ... after twap finishedper block.Change
scripts/_twap_calldata.py(new) - encodes the calldata fresh on every invocation witht0 = int(time.time()) - 60(backdated 60 s so part 0 is Ready as soon as the order is on-chain). Module docstring explicitly warns against re-hardcodingt0.scripts/e2e-onchain.sh- Action 1 now shells out to the helper rather than carrying the hex inline. Validates the output is hex-shaped before passing tocast send.docs/operations/e2e-cow-1064-prep.mdsection 2.3 step 3 - replaces the pinned blob with apython3 scripts/_twap_calldata.pyrecipe and a historical note pointing at COW-1077.docs/operations/e2e-cow-1064-prep.mdsection 4.2 recipe - getsimport time+int(time.time()) - 60fort0so the manual re-derivation flow cannot re-introduce the bug.scripts/README.mdAction 1 description updated to mention the helper.Constants in the helper (sell/buy tokens, amounts,
n,t, salt) mirror the prep doc's section 4.2; both must change in lockstep if the TWAP shape is retargeted - the helper docstring calls this out.Validation
python3 scripts/_twap_calldata.pyproduces 516-byte calldata (1034 hex chars)0x6bfae1ca(create((address,bytes32,bytes),bool))t0word reflects current epoch (verified against0x00...006a3537b5on smoke run)bash -n scripts/e2e-onchain.shparses cleanlyNo engine-side changes; this is a script-and-docs PR. No new tests because the helper is a standalone, deterministic encoder - the calldata's correctness is structurally guarded by the next E2E run: if
t0is wrong, the TWAP order never reaches Ready and the dry-run report flags it.Stack
fix/twap-calldata-helper-cow-1077→feat/ethflow-expected-excessive-validto-cow-1076(PR #49) →feat/forward-orderbook-error-cow-1075(PR #48) →feat/resolve-app-data-cow-1074(PR #47).Linear
Closes COW-1077.
AI assistance disclosure: drafted by Claude (Opus 4.7, 1M context). The original
t0 = 0calldata was itself drafted by an earlier AI session; this PR returns the encoding to a hand-validated form.