feat: API gating signer + signed context injection for take-orders#78
feat: API gating signer + signed context injection for take-orders#78hardyjosh wants to merge 1 commit into2026-04-25-fix-cifrom
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label add-to-gt-merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
0faa43e to
3e267bd
Compare
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 1 minutes and 27 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a gating signing feature: new signing module and state, config TTL with validation, Rocket startup wiring for a signer, an API injector to attach signed contexts to orders, swap route/trait plumbing to thread the injector and counterparty, test updates, and a rain.orderbook submodule bump. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Authenticated Client
participant API as Swap API Handler
participant Injector as ApiGatingInjector
participant Signer as GatingSigner
participant DS as Swap Data Source
participant Orders as Order Collection
Client->>API: POST /swap/calldata (with AuthenticatedKey)
API->>Injector: ApiGatingInjector::for_request(gating_state, key)
API->>DS: get_calldata(request, injector)
DS->>Orders: Fetch orders for pair
loop For each Order
DS->>Injector: contexts_for(order, counterparty)
Injector->>Injector: Compute order_hash = keccak256(order.abi_encode())
Injector->>Signer: sign_gating_context(counterparty, order_hash, expiry, attribution_id)
Signer->>Signer: Pack [recipient, orderHash, expiry, id] -> keccak256 -> sign
Signer-->>Injector: SignedGatingContext
Injector-->>DS: Vec<SignedContextV1>
end
DS-->>API: SwapCalldataResponse (with signed contexts)
API-->>Client: 200 OK (calldata + signatures)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
3e267bd to
3cffe91
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/types/swap.rs (1)
15-24: Consider explicit validation semantics for missingtakeron gated quote flows.Keeping
takeroptional is good for compatibility, but for gated liquidity paths this can become a silent footgun. Returning a clearApiErrorwhen gating is requested andtakeris absent would make failures deterministic for clients.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/swap.rs` around lines 15 - 24, The optional taker field on the Swap type (pub taker: Option<Address>) can silently allow gated-quote requests without a taker; add explicit validation where quotes are generated/validated (e.g., the quote creation/validation handler that checks API gating and before calling takeOrders4) to detect when a gating signature is required but swap.taker.is_none() and return a clear ApiError (or the project’s standard validation error) indicating "missing taker for gated quote". Update the handler that constructs/validates gated quotes to perform this check early and return the ApiError instead of proceeding.src/config.rs (1)
13-18: Add config-time bounds validation forgating_signature_ttl_seconds.The new field currently accepts any
u64; validating a safe range at load time avoids accidental0-second expiries or excessively long replay windows.💡 Proposed hardening change
impl Config { pub fn load(path: &Path) -> Result<Self, String> { let contents = std::fs::read_to_string(path).map_err(|e| format!("failed to read config: {e}"))?; - toml::from_str(&contents).map_err(|e| format!("failed to parse config: {e}")) + let cfg: Self = + toml::from_str(&contents).map_err(|e| format!("failed to parse config: {e}"))?; + + if cfg.gating_signature_ttl_seconds == 0 { + return Err("failed to parse config: gating_signature_ttl_seconds must be > 0".into()); + } + if cfg.gating_signature_ttl_seconds > 300 { + return Err( + "failed to parse config: gating_signature_ttl_seconds must be <= 300".into(), + ); + } + + Ok(cfg) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 13 - 18, Add a config-time bounds check for the gating_signature_ttl_seconds field so it cannot be 0 or unreasonably large: when building/parsing the Config (e.g., in Config::from_env, Config::load, or a Config::validate method), validate that gating_signature_ttl_seconds > 0 and <= 86_400 (or your chosen upper bound), and return a clear Err/ConfigError if out of range; reference the field name gating_signature_ttl_seconds in the error message so callers can locate the bad value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/rain.orderbook`:
- Line 1: The submodule pin in lib/rain.orderbook currently points at SHA
97e240997a9d35ad7166d4189cc75301e1b43345 from an unmerged branch; rebase your
branch onto the upstream default branch once rainlanguage/rain.orderbook has
merged the work, update the submodule reference in lib/rain.orderbook to the
commit that exists on upstream main (the merged SHA), and commit the updated
submodule pointer so the repo no longer references a transient branch tip;
ensure the updated submodule commit matches upstream main before pushing.
---
Nitpick comments:
In `@src/config.rs`:
- Around line 13-18: Add a config-time bounds check for the
gating_signature_ttl_seconds field so it cannot be 0 or unreasonably large: when
building/parsing the Config (e.g., in Config::from_env, Config::load, or a
Config::validate method), validate that gating_signature_ttl_seconds > 0 and <=
86_400 (or your chosen upper bound), and return a clear Err/ConfigError if out
of range; reference the field name gating_signature_ttl_seconds in the error
message so callers can locate the bad value.
In `@src/types/swap.rs`:
- Around line 15-24: The optional taker field on the Swap type (pub taker:
Option<Address>) can silently allow gated-quote requests without a taker; add
explicit validation where quotes are generated/validated (e.g., the quote
creation/validation handler that checks API gating and before calling
takeOrders4) to detect when a gating signature is required but
swap.taker.is_none() and return a clear ApiError (or the project’s standard
validation error) indicating "missing taker for gated quote". Update the handler
that constructs/validates gated quotes to perform this check early and return
the ApiError instead of proceeding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee13ba56-08bc-4eb1-bdf8-2c99dbc42ace
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
config/dev.tomlconfig/rest-api.tomldev-config.tomllib/rain.orderbooksrc/config.rssrc/main.rssrc/raindex/gating_injector.rssrc/raindex/mod.rssrc/routes/order/mod.rssrc/routes/swap/calldata.rssrc/routes/swap/mod.rssrc/routes/swap/quote.rssrc/routes/tokens.rssrc/signing.rssrc/test_helpers.rssrc/types/swap.rs
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
3cffe91 to
54023a8
Compare
|
Addressing the two nitpicks from @coderabbitai:
Explicit error for missing |
|
Tip For best results, initiate chat on the files or code changes.
TTL bounds validation — The
✏️ Learnings added
|
| tracing::info!(docs_dir = %cfg.docs_dir, "serving documentation at /docs"); | ||
|
|
||
| let rocket = match rocket(pool, rate_limiter, shared_raindex, cfg.docs_dir) { | ||
| let gating_key = match std::env::var("ST0X_GATING_SIGNER_KEY") { |
There was a problem hiding this comment.
This introduces a new required env var, but .env.example was not updated to document it. Our repo rules explicitly require using .env.example for env-var documentation:
Lines 14 to 20 in 54023a8
.env.example will miss ST0X_GATING_SIGNER_KEY and the server now exits at startup.
There was a problem hiding this comment.
Added to .env.example in 5627e0c with a comment explaining the purpose + failure mode. Thanks for the catch.
findolor
left a comment
findolor
left a comment
There was a problem hiding this comment.
Non-blocking nitpick: ApiGatingInjector construction is duplicated in both swap handlers. It works, but a small helper or service on the managed gating state would keep the route layer from assembling the same request-scoped policy object twice.
| /// calldata response embeds a signed context whose `expiry` field is | ||
| /// `now() + this`. Keep short enough that a captured signature is not | ||
| /// useful for long. The gating signer private key is read from the | ||
| /// `ST0X_GATING_SIGNER_KEY` env var and never appears in this file. |
There was a problem hiding this comment.
ST0X_GATING_SIGNER_KEY is now required at startup, but .env.example was not updated to document it. That will break fresh setup/deploy flows that rely on .env.example, and it violates the repo rule to use .env.example for documenting env vars.
There was a problem hiding this comment.
Same fix — .env.example updated in 5627e0c.
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
54023a8 to
5627e0c
Compare
|
Both review points addressed in 5627e0c:
@findolor ready for another look. |
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
5627e0c to
c7e01f7
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/signing.rs (1)
66-67: Avoid extra temporary allocations in the signing hot path.
flat_map(|b| b.as_slice().to_vec())allocates once per context word. Preallocating once and extending from slices is cheaper and a bit clearer here.♻️ Suggested refactor
- let packed: Vec<u8> = context.iter().flat_map(|b| b.as_slice().to_vec()).collect(); + let mut packed = Vec::with_capacity(context.len() * 32); + for word in &context { + packed.extend_from_slice(word.as_slice()); + } let hash = keccak256(&packed);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/signing.rs` around lines 66 - 67, The current hot path builds `packed` by calling `context.iter().flat_map(|b| b.as_slice().to_vec()).collect()`, which allocates a Vec per context word; instead, compute the total capacity from `context` (e.g., sum of slice lengths), create a single `Vec<u8>` with that capacity, and then extend it from each slice (using `extend_from_slice`) inside a loop or iterator so only one allocation occurs before calling `keccak256(&packed)`; update the code around `packed`, `context`, and the call to `keccak256` accordingly.src/raindex/gating_injector.rs (1)
53-55: Add more context to signer-failure logs.Including
order_hash,counterparty, andattribution_idhere would make signer outages much easier to diagnose. Right now this can degrade into an apparent “no liquidity” case from the caller side with little correlation data.📈 Suggested logging tweak
- tracing::error!(error = %e, "gating signer failed to sign context; order will fail on-chain gating"); + tracing::error!( + error = %e, + %counterparty, + %order_hash, + attribution_id = %self.attribution_id, + "gating signer failed to sign context; order will fail on-chain gating" + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/raindex/gating_injector.rs` around lines 53 - 55, The signer failure log in the Err(e) branch currently only logs the error; update the tracing::error! call in the Err(e) handler (the block that currently logs "gating signer failed to sign context; order will fail on-chain gating") to include the order's identifying fields—order_hash, counterparty, and attribution_id—as structured fields (e.g., order_hash = %order_hash, counterparty = %counterparty, attribution_id = %attribution_id) so the log records those values alongside the error for easier diagnosis.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/raindex/gating_injector.rs`:
- Around line 53-55: The signer failure log in the Err(e) branch currently only
logs the error; update the tracing::error! call in the Err(e) handler (the block
that currently logs "gating signer failed to sign context; order will fail
on-chain gating") to include the order's identifying fields—order_hash,
counterparty, and attribution_id—as structured fields (e.g., order_hash =
%order_hash, counterparty = %counterparty, attribution_id = %attribution_id) so
the log records those values alongside the error for easier diagnosis.
In `@src/signing.rs`:
- Around line 66-67: The current hot path builds `packed` by calling
`context.iter().flat_map(|b| b.as_slice().to_vec()).collect()`, which allocates
a Vec per context word; instead, compute the total capacity from `context`
(e.g., sum of slice lengths), create a single `Vec<u8>` with that capacity, and
then extend it from each slice (using `extend_from_slice`) inside a loop or
iterator so only one allocation occurs before calling `keccak256(&packed)`;
update the code around `packed`, `context`, and the call to `keccak256`
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9255c301-539d-4fe1-a434-c6d6e89d5803
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.env.exampleconfig/dev.tomlconfig/rest-api.tomldev-config.tomllib/rain.orderbooksrc/config.rssrc/main.rssrc/raindex/gating_injector.rssrc/raindex/mod.rssrc/routes/order/mod.rssrc/routes/swap/calldata.rssrc/routes/swap/mod.rssrc/routes/swap/quote.rssrc/routes/tokens.rssrc/signing.rssrc/test_helpers.rssrc/types/swap.rs
✅ Files skipped from review due to trivial changes (7)
- dev-config.toml
- config/dev.toml
- config/rest-api.toml
- .env.example
- src/routes/tokens.rs
- src/routes/order/mod.rs
- src/raindex/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/types/swap.rs
- src/routes/swap/quote.rs
- src/main.rs
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
c7e01f7 to
a2272b5
Compare
|
CodeRabbit round 2 addressed in a2272b5:
Submodule bumped to 5a1d92ae (latest raindex branch head with the single-take + clone-defer + output-index fixes). |
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
a2272b5 to
f7635f8
Compare
## Motivation The rain-solver arbs between our Raindex book and pools we run on Hyperliquid/other venues, which quickly drains the shared USDC buy-side liquidity on public pyth-priced orders. A single arb can burn $10k of buy-side liquidity in one trade, leaving API users of st0x.io with no available liquidity even though they only need a $1k fill. We want a separate pot of USDC that's only reachable through the API, so arb flow can't touch it. There's no native way to gate a vault, so the gate needs to live in the strategy itself. ## Solution New `gated-pyth.rain` variant of the fixed-spread pyth strategy. Reuses the same pyth-priced, multiplier-spread IO calculation but adds four SignedContext assertions in `calculate-io`: - `signer<0>()` must equal the `api-signer` binding (the trusted EOA whose key the off-chain API holds). - `signed-context<0 0>()` must equal `order-counterparty()` — the signature is bound to the specific taker. - `signed-context<0 1>()` must equal `order-hash()` — the signature is bound to this order. - `signed-context<0 2>()` must be `>= now()` — the authorisation is not expired. A fourth entry `<0 3>` carries `keccak256(api_key_id)` — signed but not asserted on-chain. It exists purely so trades can be attributed back to the calling API key off-chain via event indexing. The registry pointer file entry is added using the existing stale commit hash. A follow-up "Update registry hash" commit post-merge will refresh all entries to the new head. Companion PRs: - rainlanguage/raindex#2547 — adds `SignedContextInjector` so rain.orderbook can receive caller-supplied signed contexts. - ST0x-Technology/st0x.rest.api#78 — API signer that produces the `SignedContextV1` this strategy asserts. ## Checks By submitting this for review, I'm confirming I've done the following: - [ ] added comprehensive test coverage for any changes in logic - [x] made this PR as small as possible - [x] linked any relevant issues or PRs - [ ] included screenshots (if this involves a change to a front-end/dashboard) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## New Features - Introduced a Pyth oracle-based strategy with API-authenticated execution - Enables configurable order pricing using Pyth price feeds - Supports both standard and inverse price modes - Includes adjustable parameters for price multipliers and oracle timeouts <!-- end of auto-generated comment: release notes by coderabbit.ai -->
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
947927d to
3e7b7ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/signing.rs`:
- Around line 19-23: GatingSigner currently derives Debug and therefore may
accidentally expose secret material (the inner PrivateKeySigner created from
ST0X_GATING_SIGNER_KEY); replace the derived Debug with a manual Debug impl or a
redacted struct display that omits the private key and only outputs safe info
(e.g., the public address or a masked identifier). Specifically, remove
#[derive(Debug)] from GatingSigner and implement core::fmt::Debug for
GatingSigner so the fmt method only prints the public address (or a fixed
redacted string) and never reveals inner or PrivateKeySigner internals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fff0bad2-1c70-4f32-aef9-5e0acd5e9f91
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.env.exampleconfig/dev.tomlconfig/rest-api.tomldev-config.tomllib/rain.orderbooksrc/config.rssrc/main.rssrc/raindex/gating_injector.rssrc/raindex/mod.rssrc/routes/order/mod.rssrc/routes/swap/calldata.rssrc/routes/swap/mod.rssrc/routes/swap/quote.rssrc/routes/tokens.rssrc/signing.rssrc/test_helpers.rssrc/types/swap.rs
✅ Files skipped from review due to trivial changes (5)
- config/rest-api.toml
- src/routes/tokens.rs
- dev-config.toml
- src/routes/order/mod.rs
- src/raindex/mod.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- config/dev.toml
- .env.example
- lib/rain.orderbook
- src/config.rs
- src/routes/swap/quote.rs
- src/routes/swap/mod.rs
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
2 times, most recently
from
452c68e to
e2c6c61
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/signing.rs`:
- Around line 62-96: The code in sign_gating_context currently uses
self.inner.sign_message(hash.as_slice()), which applies the EIP-191 prefix;
replace that call with the signer method that signs the raw 32-byte hash (e.g.,
sign_hash, sign_raw_hash, or the underlying low-level signing function on
self.inner) or construct the signature over hash bytes manually so no
"\x19Ethereum Signed Message..." prefix is added; keep the same error mapping to
GatingSignerError::SignFailed and return SignedGatingContext with
signature.as_bytes().to_vec() as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ef0ef4f-faa4-4eea-8664-3dc0839fb2ea
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.env.exampleconfig/dev.tomlconfig/rest-api.tomldev-config.tomllib/rain.orderbooksrc/config.rssrc/main.rssrc/raindex/gating_injector.rssrc/raindex/mod.rssrc/routes/order/mod.rssrc/routes/orders/get_by_owner.rssrc/routes/orders/get_by_token.rssrc/routes/swap/calldata.rssrc/routes/swap/mod.rssrc/routes/swap/quote.rssrc/routes/tokens.rssrc/signing.rssrc/test_helpers.rssrc/types/swap.rs
✅ Files skipped from review due to trivial changes (10)
- config/dev.toml
- .env.example
- config/rest-api.toml
- dev-config.toml
- src/routes/order/mod.rs
- src/routes/orders/get_by_token.rs
- lib/rain.orderbook
- src/routes/tokens.rs
- src/routes/orders/get_by_owner.rs
- src/raindex/mod.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- src/config.rs
- src/routes/swap/mod.rs
| /// Sign `[recipient, orderHash, expiry, id]` as a four-entry signed | ||
| /// context. The signature is EIP-191 over `keccak256(abi.encodePacked(context))`, | ||
| /// matching `LibContext.build` in the orderbook contract. | ||
| pub async fn sign_gating_context( | ||
| &self, | ||
| recipient: Address, | ||
| order_hash: B256, | ||
| expiry: u64, | ||
| id: B256, | ||
| ) -> Result<SignedGatingContext, GatingSignerError> { | ||
| let context: [FixedBytes<32>; 4] = [ | ||
| address_to_b256(recipient), | ||
| order_hash, | ||
| u64_to_b256(expiry), | ||
| id, | ||
| ]; | ||
|
|
||
| let mut packed = Vec::with_capacity(context.len() * 32); | ||
| for word in &context { | ||
| packed.extend_from_slice(word.as_slice()); | ||
| } | ||
| let hash = keccak256(&packed); | ||
| let signature = self | ||
| .inner | ||
| .sign_message(hash.as_slice()) | ||
| .await | ||
| .map_err(|e| GatingSignerError::SignFailed(e.to_string()))?; | ||
|
|
||
| Ok(SignedGatingContext { | ||
| signer: self.address(), | ||
| context: context.to_vec(), | ||
| signature: signature.as_bytes().to_vec(), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the signed-context verification in the raindex submodule and confirm
# whether it applies EIP-191 prefix before ecrecover.
fd -t f -e sol | xargs rg -n -C5 -P '(toEthSignedMessageHash|Ethereum Signed Message|ecrecover|ECDSA\.recover)' 2>/dev/null | rg -B2 -A8 -i 'signedContext|LibContext|signer|build'Repository: ST0x-Technology/st0x.rest.api
Length of output: 55
🌐 Web query:
In rainlanguage rain.orderbook LibContext or signed context verification, does the signer recovery use EIP-191 toEthSignedMessageHash prefix, or raw keccak256 of abi.encodePacked?
💡 Result:
In Rain Language's rain.orderbook LibContext or signed context verification, the signer recovery uses raw keccak256 of abi.encodePacked, not the EIP-191 toEthSignedMessageHash prefix. No sources explicitly confirm EIP-191 usage; general Ethereum signature verification patterns and lack of EIP-191 mentions in Rain Orderbook docs and repo suggest raw hash verification typical for on-chain permit-like or order signing systems.
Citations:
Remove EIP-191 prefix: sign the raw hash directly without sign_message().
The on-chain LibContext verifier in rain.orderbook expects signatures over the raw keccak256(abi.encodePacked(context)), not the EIP-191-prefixed variant. The current code calls self.inner.sign_message(hash.as_slice()), which applies the "\x19Ethereum Signed Message:\n32" prefix, causing a signature mismatch. Every gated take will fail on-chain verification despite local unit tests passing.
Sign the raw hash directly instead—either by calling the underlying signing method without the EIP-191 wrapper, or by constructing the signature manually.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/signing.rs` around lines 62 - 96, The code in sign_gating_context
currently uses self.inner.sign_message(hash.as_slice()), which applies the
EIP-191 prefix; replace that call with the signer method that signs the raw
32-byte hash (e.g., sign_hash, sign_raw_hash, or the underlying low-level
signing function on self.inner) or construct the signature over hash bytes
manually so no "\x19Ethereum Signed Message..." prefix is added; keep the same
error mapping to GatingSignerError::SignFailed and return SignedGatingContext
with signature.as_bytes().to_vec() as before.
There was a problem hiding this comment.
Disagree — the on-chain verifier does use EIP-191. See lib/rain.interpreter/lib/rain.interpreter.interface/src/lib/caller/LibContext.sol:204-210:
!SignatureChecker.isValidSignatureNow(
signedContexts[i].signer,
MessageHashUtils.toEthSignedMessageHash(
LibHashNoAlloc.hashWords(signedContexts[i].context)
),
signedContexts[i].signature
)MessageHashUtils.toEthSignedMessageHash applies the "\x19Ethereum Signed Message:\n32" prefix, which matches sign_message() on alloy's PrivateKeySigner. Empirically confirmed by the smoke test — a real on-chain take landed against the gated-pyth order with signer<0>() recovering as our API signer. If the prefixes mismatched, the strategy's :ensure(equal-to(signer<0>() api-signer)) would have reverted.
The AI agent's web query came back with an unsourced negative ("no sources explicitly confirm EIP-191"), but the source code does.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the LibContext.sol content the user cited
fd -t f 'LibContext.sol' | xargs grep -n -A10 'toEthSignedMessageHash\|isValidSignatureNow' 2>/dev/null | head -60
Length of output: 0
@hardyjosh, you're absolutely right — I apologize for the incorrect review comment. The cited Solidity code in LibContext.sol is definitive:
!SignatureChecker.isValidSignatureNow(
signedContexts[i].signer,
MessageHashUtils.toEthSignedMessageHash(
LibHashNoAlloc.hashWords(signedContexts[i].context)
),
signedContexts[i].signature
)MessageHashUtils.toEthSignedMessageHash applies the "\x19Ethereum Signed Message:\n32" EIP-191 prefix, which is exactly what alloy's sign_message() produces on the Rust side. The implementation in sign_gating_context is correct. My original flag was based on an unsourced web query that returned a speculative negative — I should have located the actual contract source before raising it. The empirical confirmation (on-chain take succeeding with the correct signer recovery) is the strongest possible evidence.
✏️ Learnings added
Learnt from: hardyjosh
Repo: ST0x-Technology/st0x.rest.api PR: 78
File: src/signing.rs:62-96
Timestamp: 2026-04-25T02:05:52.379Z
Learning: In rain.orderbook (rainlanguage), LibContext.sol verifies signed contexts using EIP-191: it calls `MessageHashUtils.toEthSignedMessageHash(LibHashNoAlloc.hashWords(signedContexts[i].context))` before `SignatureChecker.isValidSignatureNow`. This means the signing side must use the EIP-191-prefixed variant (e.g., alloy's `sign_message()` on `PrivateKeySigner`), NOT a raw hash signing method. In st0x.rest.api, `GatingSigner::sign_gating_context` correctly uses `self.inner.sign_message(hash.as_slice())` for this reason.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: hardyjosh
Repo: ST0x-Technology/st0x.rest.api PR: 0
File: :0-0
Timestamp: 2026-04-14T18:38:39.699Z
Learning: In st0x.rest.api (Rust/Rocket), `SwapQuoteRequest.taker: Option<Address>` is intentionally optional. When omitted, `Address::ZERO` is passed as the counterparty to `ApiGatingInjector`. Gated orders (which use an on-chain `:ensure(equal-to(signed-context<0 0>() order-counterparty()))` check) are then filtered out of candidate selection during simulation because the ZERO-signed context won't satisfy the check. Non-gated orders still price normally. Clients that want gated orders included in the quote must supply their `taker` address. The API must NOT try to detect whether orders are gated, as that would require knowledge of strategy internals.
Learnt from: hardyjosh
Repo: ST0x-Technology/st0x.rest.api PR: 0
File: :0-0
Timestamp: 2026-04-14T18:38:39.699Z
Learning: In st0x.rest.api (Rust/Rocket), `Config.gating_signature_ttl_seconds` is validated in `Config::load` to reject `0` and any value greater than 300 (hard ceiling). This is a security control: the signed gating context is replayable until `expiry = now + ttl`, so the TTL bounds directly limit replay window exposure.
Learnt from: findolor
Repo: ST0x-Technology/st0x.rest.api PR: 18
File: src/fairings/rate_limiter.rs:158-203
Timestamp: 2026-02-13T09:37:37.357Z
Learning: When multiple guards or layers write to the same cached rate-limit information, document and enforce the execution order so that the client-visible headers reflect the intended quota (global vs per-key). In this repo, rate-limiter.rs and auth-related guards intentionally write to the same CachedRateLimitInfo entry; ensure the per-key quota is what’s surfaced to clients and that the order is explicit in route signatures. Include tests that verify the final headers reflect the intended quota and that writes are intentional and auditable.
Introduces a gating signer that produces SignedContextV1 entries for every candidate order in /v1/swap/calldata and /v1/swap/quote. The signed payload is [recipient, orderHash, expiry, id] where id is keccak256(api_key_id), retained for off-chain attribution. The signer key is loaded from env (ST0X_GATING_SIGNER_KEY, required at startup) and never written to disk; TTL is `gating_signature_ttl_seconds` in config. Architecture: - New `signing` module: EIP-191 signer adapted from st0x-oracle-server, plus `GatingState` held in rocket-managed state. - New `raindex::gating_injector::ApiGatingInjector` implements the upstream `SignedContextInjector` trait: per-order contexts_for() signs `[taker, keccak256(abi.encode(order)), expiry, id]` and returns a single SignedContextV1 appended after any oracle-discovered contexts. - `SwapDataSource` trait grows `counterparty` + `injector` params. `RaindexSwapDataSource` forwards to `get_take_orders_calldata_with_injector`. - `SwapQuoteRequest` gains an optional `taker` field (backward compat; gated orders need it, others don't). **Merge order**: this PR depends on rainlanguage/raindex#2547. The submodule is pinned to that branch's head; after #2547 merges, rebase this branch onto main and bump `lib/rain.orderbook` to the new main head.
hardyjosh
force-pushed
the
2026-04-14-api-gated-signing
branch
from
e2c6c61 to
78edf38
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
3 participants
Caution
Depends on rainlanguage/raindex#2550 — must merge first. After it lands, bump
lib/rain.orderbookin this PR to main head before merge (the submodule bump commit here currently points at the #2550 branch head).Motivation
The rain-solver arbs between our Raindex book and pools we run on other venues, which can drain our shared USDC buy-side on public pyth orders in a single trade. API users of st0x.io then see no liquidity even though they only need modest fills.
To fix this without competing with arb flow, we want a separate USDC pot reserved for API callers. There is no native way to gate a vault, so the gate lives in the strategy and is enforced via a
SignedContextV1entry that only the API can produce.Companion PRs:
SignedContextInjectorso callers can contribute signed contexts. (merged)msg.senderreachescalculate-ioas the real counterparty. (required — see caution block above)gated-pyth.rainstrategy that asserts on the signed context.Solution
signingmodule: EIP-191 signer adapted fromst0x-oracle-server, plusGatingStateheld as rocket-managed state. Signs[recipient, orderHash, expiry, id]as four bytes32 entries, whereid = keccak256(api_key_id)for off-chain attribution.ST0X_GATING_SIGNER_KEYat startup (required, never written to disk). TTL isgating_signature_ttl_secondsin config — each swap call embedsexpiry = now() + ttl.raindex::gating_injector::ApiGatingInjectorimplements the upstreamSignedContextInjectortrait. Per-ordercontexts_for()signs over the canonical order hash (keccak256(abi.encode(order))) and returns a singleSignedContextV1for the injector pipeline to append after any oracle-discovered contexts.SwapDataSourcetrait growscounterparty: Address+&dyn SignedContextInjectorparams.RaindexSwapDataSourceforwards to the newget_take_orders_calldata_with_injector.SwapQuoteRequestgains optionaltaker: Option<Address>(backward-compatible — gated orders need it, public orders don't).GatingState+ the authenticated key'skey_idper request.Verified locally:
cargo checkclean,cargo test --bin st0x_rest_api— 143 passed,cargo fmt.Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Chores