feat(filler-sdk): standalone client SDK for external fillers#6
Open
VAIBHAVJINDAL3012 wants to merge 4 commits into
Open
feat(filler-sdk): standalone client SDK for external fillers#6VAIBHAVJINDAL3012 wants to merge 4 commits into
VAIBHAVJINDAL3012 wants to merge 4 commits into
Conversation
Add `pswap-filler-sdk`: the crate external DEXes depend on to integrate with the solver's RFQ router. It owns the miden-free wire protocol (so the solver and SDK can't drift); the default build pulls no miden. Async `FillerClient` (connect/subscribe/quote/next_event); on-chain decode helpers behind an opt-in `consume` feature that pulls miden-protocol/ standards only (not miden-client). See docs/adr/0002-filler-sdk.md and docs/filler-integration.md.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdds a new standalone ChangesPSWAP Filler SDK
Sequence Diagram(s)sequenceDiagram
participant Application
participant FillerClient
participant pump
participant Router
Application->>FillerClient: connect(url, token)
FillerClient->>Router: WebSocket upgrade with Bearer token
FillerClient->>pump: spawn background task
Application->>FillerClient: subscribe()/quote()
FillerClient->>pump: enqueue ClientMsg
pump->>Router: send JSON text frame
Router->>pump: deliver ServerMsg JSON frame
pump->>Application: next_event() yields FillerEvent
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/filler-sdk/Cargo.toml`:
- Line 20: The tokio-tungstenite dependency is missing TLS support, so wss://
connections will fail at runtime; update the dependency in Cargo.toml to enable
the rustls-tls-webpki-roots feature. Use the existing tokio-tungstenite entry as
the target and keep the change scoped to the dependency declaration so the
MaybeTlsStream-based connection code can handle secure WebSocket endpoints
correctly.
In `@crates/filler-sdk/src/client.rs`:
- Around line 147-155: The connect method in client::connect currently attaches
the Authorization bearer token before verifying the websocket scheme, which can
leak credentials over plaintext ws:// URLs. Update connect to reject non-TLS
websocket endpoints by default (only allow wss://), and if a local/dev escape
hatch is needed, gate it explicitly before building the request and inserting
the Authorization header so the token is never sent over plaintext.
- Around line 218-249: The outbound write path in the client pump can stall the
whole tokio::select! loop because sink.send() is awaited without a bound, which
blocks stream.next() and violates the non-blocking contract. Update the loop in
the client message pump to apply a write timeout around the
sink.send(Message::Text(...)) call, and on timeout treat it as a send failure
path that breaks or otherwise closes the pump cleanly. Keep the fix localized to
the pump loop and preserve the existing handling of serialization errors,
inbound decode, and FillerEvent forwarding.
- Around line 90-98: Both pump queues are unbounded, so FillerSender and the
channel created with mpsc::unbounded_channel can grow without limit and OOM the
process. Replace the unbounded sender/channel with a bounded mpsc queue and
propagate backpressure by making FillerSender::send return a “queue full” error
when the buffer is saturated. Update the pump setup that constructs the
sender/receiver pair to use the bounded capacity consistently, and adjust any
call sites that assume sends can never fail except on disconnect.
In `@crates/filler-sdk/src/consume.rs`:
- Around line 56-73: In PswapTerms::from_note, the offered and requested amount
fields are currently derived from FungibleAsset::amount() using a generic
conversion, but amount() returns an AssetAmount wrapper, so update the
extraction to use the explicit as_u64() accessor instead. Keep the rest of the
parsing logic in PswapNote::try_from, offered_asset(), and
storage().requested_asset() unchanged, and make sure both offered_amount and
requested_amount are populated from the wrapper’s u64 value via as_u64().
🪄 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 Plus
Run ID: 5ae846ed-7afa-4282-b3d4-6d8bb2ec4bd2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlcrates/filler-sdk/Cargo.tomlcrates/filler-sdk/README.mdcrates/filler-sdk/src/client.rscrates/filler-sdk/src/consume.rscrates/filler-sdk/src/lib.rscrates/filler-sdk/src/protocol.rscrates/solver/pswap-attachment-corruption-report.mddocs/adr/0002-filler-sdk.mddocs/filler-integration.md
| serde = { version = "1.0", features = ["derive"] } | ||
| serde_json = "1.0" | ||
| tokio = { version = "1.40", features = ["rt", "macros", "sync", "time", "net"] } | ||
| tokio-tungstenite = "0.29" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🌐 Web query:
What features does tokio-tungstenite 0.29 enable by default, and what feature flags enable rustls or native-tls for wss connections?
💡 Result:
In tokio-tungstenite version 0.29, the default features enabled are connect and handshake [1][2]. These default features collectively enable the stream, tokio/net, and tungstenite/handshake features [1][2]. For secure WebSocket (wss://) connections, the following feature flags are used to enable TLS support, neither of which is enabled by default [3][4]: - native-tls: Enables support using the native-tls crate [1]. - rustls-tls-native-roots: Enables support using rustls with system-native certificate roots [1][3]. - rustls-tls-webpki-roots: Enables support using rustls with webpki-roots certificate store [1][3].
Citations:
- 1: https://docs.rs/crate/tokio-tungstenite/0.29.0/features
- 2: https://docs.rs/crate/tokio-tungstenite/latest/source/Cargo.toml.orig
- 3: https://docs.rs/crate/tokio-tungstenite/latest
- 4: https://github.com/snapview/tokio-tungstenite/blob/master/README.md
tokio-tungstenite lacks TLS features — wss:// connections will fail.
The dependency currently uses default features, which are connect and handshake. TLS support is not included by default. Since the codebase uses MaybeTlsStream to handle connections that may include wss:// endpoints, the connection will fail at runtime without an explicit TLS feature enabled.
Update the dependency to include rustls-tls-webpki-roots:
🔒 Proposed change
-tokio-tungstenite = "0.29"
+tokio-tungstenite = { version = "0.29", features = ["rustls-tls-webpki-roots"] }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| tokio-tungstenite = "0.29" | |
| tokio-tungstenite = { version = "0.29", features = ["rustls-tls-webpki-roots"] } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/filler-sdk/Cargo.toml` at line 20, The tokio-tungstenite dependency is
missing TLS support, so wss:// connections will fail at runtime; update the
dependency in Cargo.toml to enable the rustls-tls-webpki-roots feature. Use the
existing tokio-tungstenite entry as the target and keep the change scoped to the
dependency declaration so the MaybeTlsStream-based connection code can handle
secure WebSocket endpoints correctly.
Comment on lines
+90
to
+98
| pub struct FillerSender { | ||
| tx: mpsc::UnboundedSender<ClientMsg>, | ||
| } | ||
|
|
||
| impl FillerSender { | ||
| /// Send a raw protocol message. Errors only if the connection is gone. | ||
| pub fn send(&self, msg: ClientMsg) -> Result<()> { | ||
| self.tx.send(msg).map_err(|_| anyhow!("router connection closed")) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n src/crates/filler-sdk/src/client.rs | sed -n '80,110p'Repository: inicio-labs/solver
Length of output: 224
🏁 Script executed:
fd client.rs --type fRepository: inicio-labs/solver
Length of output: 188
🏁 Script executed:
cat -n crates/filler-sdk/src/client.rs | sed -n '80,110p'Repository: inicio-labs/solver
Length of output: 1648
🏁 Script executed:
cat -n crates/filler-sdk/src/client.rs | sed -n '150,170p'Repository: inicio-labs/solver
Length of output: 1148
Bound the pump queues to prevent unbounded memory growth.
Both queues are unbounded (mpsc::UnboundedSender at line 91, mpsc::unbounded_channel at lines 161-162). If the socket stalls or events are produced faster than callers drain them, this SDK can OOM the filler process instead of applying backpressure or returning “queue full”.
♻️ Directional fix
+const OUTBOUND_QUEUE_CAPACITY: usize = 1024;
+const EVENT_QUEUE_CAPACITY: usize = 1024;
+
#[derive(Clone)]
pub struct FillerSender {
- tx: mpsc::UnboundedSender<ClientMsg>,
+ tx: mpsc::Sender<ClientMsg>,
}
impl FillerSender {
/// Send a raw protocol message. Errors only if the connection is gone.
pub fn send(&self, msg: ClientMsg) -> Result<()> {
- self.tx.send(msg).map_err(|_| anyhow!("router connection closed"))
+ self.tx.try_send(msg).map_err(|e| {
+ if e.is_full() {
+ anyhow!("router outbound queue full")
+ } else {
+ anyhow!("router connection closed")
+ }
+ })
}- let (out_tx, out_rx) = mpsc::unbounded_channel::<ClientMsg>();
- let (ev_tx, ev_rx) = mpsc::unbounded_channel::<FillerEvent>();
+ let (out_tx, out_rx) = mpsc::channel::<ClientMsg>(OUTBOUND_QUEUE_CAPACITY);
+ let (ev_tx, ev_rx) = mpsc::channel::<FillerEvent>(EVENT_QUEUE_CAPACITY);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub struct FillerSender { | |
| tx: mpsc::UnboundedSender<ClientMsg>, | |
| } | |
| impl FillerSender { | |
| /// Send a raw protocol message. Errors only if the connection is gone. | |
| pub fn send(&self, msg: ClientMsg) -> Result<()> { | |
| self.tx.send(msg).map_err(|_| anyhow!("router connection closed")) | |
| } | |
| const OUTBOUND_QUEUE_CAPACITY: usize = 1024; | |
| const EVENT_QUEUE_CAPACITY: usize = 1024; | |
| pub struct FillerSender { | |
| tx: mpsc::Sender<ClientMsg>, | |
| } | |
| impl FillerSender { | |
| /// Send a raw protocol message. Errors only if the connection is gone. | |
| pub fn send(&self, msg: ClientMsg) -> Result<()> { | |
| self.tx.try_send(msg).map_err(|e| { | |
| if e.is_full() { | |
| anyhow!("router outbound queue full") | |
| } else { | |
| anyhow!("router connection closed") | |
| } | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/filler-sdk/src/client.rs` around lines 90 - 98, Both pump queues are
unbounded, so FillerSender and the channel created with mpsc::unbounded_channel
can grow without limit and OOM the process. Replace the unbounded sender/channel
with a bounded mpsc queue and propagate backpressure by making
FillerSender::send return a “queue full” error when the buffer is saturated.
Update the pump setup that constructs the sender/receiver pair to use the
bounded capacity consistently, and adjust any call sites that assume sends can
never fail except on disconnect.
Comment on lines
+147
to
+155
| pub async fn connect(url: &str, token: &str) -> Result<Self> { | ||
| let mut req = url | ||
| .into_client_request() | ||
| .with_context(|| format!("invalid router url: {url}"))?; | ||
| let bearer = format!("Bearer {token}"); | ||
| req.headers_mut().insert( | ||
| "Authorization", | ||
| bearer.parse().context("token has invalid header characters")?, | ||
| ); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject plaintext websocket URLs before attaching the bearer token.
connect currently sends Authorization: Bearer ... to any accepted URL, so a ws:// endpoint exposes the token in cleartext. Prefer wss:// by default, with an explicit local/dev escape hatch if needed.
🛡️ Proposed guard
let mut req = url
.into_client_request()
.with_context(|| format!("invalid router url: {url}"))?;
+ let is_local_dev = matches!(
+ req.uri().host(),
+ Some("localhost" | "127.0.0.1" | "::1" | "[::1]")
+ );
+ if req.uri().scheme_str() != Some("wss") && !is_local_dev {
+ bail!("refusing to send bearer token over non-TLS websocket; use wss://");
+ }
let bearer = format!("Bearer {token}");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub async fn connect(url: &str, token: &str) -> Result<Self> { | |
| let mut req = url | |
| .into_client_request() | |
| .with_context(|| format!("invalid router url: {url}"))?; | |
| let bearer = format!("Bearer {token}"); | |
| req.headers_mut().insert( | |
| "Authorization", | |
| bearer.parse().context("token has invalid header characters")?, | |
| ); | |
| pub async fn connect(url: &str, token: &str) -> Result<Self> { | |
| let mut req = url | |
| .into_client_request() | |
| .with_context(|| format!("invalid router url: {url}"))?; | |
| let is_local_dev = matches!( | |
| req.uri().host(), | |
| Some("localhost" | "127.0.0.1" | "::1" | "[::1]") | |
| ); | |
| if req.uri().scheme_str() != Some("wss") && !is_local_dev { | |
| bail!("refusing to send bearer token over non-TLS websocket; use wss://"); | |
| } | |
| let bearer = format!("Bearer {token}"); | |
| req.headers_mut().insert( | |
| "Authorization", | |
| bearer.parse().context("token has invalid header characters")?, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/filler-sdk/src/client.rs` around lines 147 - 155, The connect method
in client::connect currently attaches the Authorization bearer token before
verifying the websocket scheme, which can leak credentials over plaintext ws://
URLs. Update connect to reject non-TLS websocket endpoints by default (only
allow wss://), and if a local/dev escape hatch is needed, gate it explicitly
before building the request and inserting the Authorization header so the token
is never sent over plaintext.
Comment on lines
+218
to
+249
| loop { | ||
| tokio::select! { | ||
| // Outbound: serialize and write. A serialization failure is a bug, | ||
| // not a wire condition — skip the message rather than tear down. | ||
| out = out_rx.recv() => match out { | ||
| Some(msg) => { | ||
| let txt = match serde_json::to_string(&msg) { | ||
| Ok(t) => t, | ||
| Err(e) => { tracing::error!(error = %e, "serialize ClientMsg"); continue; } | ||
| }; | ||
| if sink.send(Message::Text(txt.into())).await.is_err() { | ||
| break; | ||
| } | ||
| } | ||
| None => break, // all senders dropped → close | ||
| }, | ||
| // Inbound: decode and forward as an event. | ||
| item = stream.next() => match item { | ||
| Some(Ok(Message::Text(t))) => { | ||
| match serde_json::from_str::<ServerMsg>(t.as_str()) { | ||
| Ok(m) => { | ||
| if ev_tx.send(FillerEvent::from(m)).is_err() { | ||
| break; // receiver dropped | ||
| } | ||
| } | ||
| Err(e) => tracing::warn!(error = %e, "drop undecodable server frame"), | ||
| } | ||
| } | ||
| Some(Ok(Message::Close(_))) | Some(Err(_)) | None => break, | ||
| Some(Ok(_)) => {} // ignore binary/ping/pong | ||
| }, | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n crates/filler-sdk/src/client.rs | sed -n '200,260p'Repository: inicio-labs/solver
Length of output: 2922
🏁 Script executed:
cat -n crates/filler-sdk/src/client.rs | sed -n '1,50p'Repository: inicio-labs/solver
Length of output: 2543
🏁 Script executed:
grep -n "time::timeout\|tokio::time" crates/filler-sdk/src/client.rsRepository: inicio-labs/solver
Length of output: 232
Prevent stalls in outbound writes from blocking inbound event processing.
The tokio::select! loop currently pauses polling of the inbound stream (stream.next()) while awaiting a long-running sink.send() call. This contradicts the stated contract at lines 6–7 that "two directions never block each other" and risks dropping or delaying critical server frames if the network stalls.
Add a write timeout to bound this blocking window and ensure the pump remains responsive to inbound events:
Recommended fix
+ use tokio::time::{timeout, Duration};
+
loop {
tokio::select! {
// ...
+ match timeout(Duration::from_secs(10), sink.send(Message::Text(txt.into()))).await {
+ Ok(Ok(())) => {}
+ Ok(Err(e)) => { tracing::warn!(error = %e, "router websocket write failed"); break; }
+ Err(_) => { tracing::warn!("router websocket write timed out"); break; }
+ }
- if sink.send(Message::Text(txt.into())).await.is_err() {
- break;
- }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| loop { | |
| tokio::select! { | |
| // Outbound: serialize and write. A serialization failure is a bug, | |
| // not a wire condition — skip the message rather than tear down. | |
| out = out_rx.recv() => match out { | |
| Some(msg) => { | |
| let txt = match serde_json::to_string(&msg) { | |
| Ok(t) => t, | |
| Err(e) => { tracing::error!(error = %e, "serialize ClientMsg"); continue; } | |
| }; | |
| if sink.send(Message::Text(txt.into())).await.is_err() { | |
| break; | |
| } | |
| } | |
| None => break, // all senders dropped → close | |
| }, | |
| // Inbound: decode and forward as an event. | |
| item = stream.next() => match item { | |
| Some(Ok(Message::Text(t))) => { | |
| match serde_json::from_str::<ServerMsg>(t.as_str()) { | |
| Ok(m) => { | |
| if ev_tx.send(FillerEvent::from(m)).is_err() { | |
| break; // receiver dropped | |
| } | |
| } | |
| Err(e) => tracing::warn!(error = %e, "drop undecodable server frame"), | |
| } | |
| } | |
| Some(Ok(Message::Close(_))) | Some(Err(_)) | None => break, | |
| Some(Ok(_)) => {} // ignore binary/ping/pong | |
| }, | |
| } | |
| use tokio::time::{timeout, Duration}; | |
| loop { | |
| tokio::select! { | |
| // Outbound: serialize and write. A serialization failure is a bug, | |
| // not a wire condition — skip the message rather than tear down. | |
| out = out_rx.recv() => match out { | |
| Some(msg) => { | |
| let txt = match serde_json::to_string(&msg) { | |
| Ok(t) => t, | |
| Err(e) => { tracing::error!(error = %e, "serialize ClientMsg"); continue; } | |
| }; | |
| match timeout(Duration::from_secs(10), sink.send(Message::Text(txt.into()))).await { | |
| Ok(Ok(())) => {} | |
| Ok(Err(e)) => { tracing::warn!(error = %e, "router websocket write failed"); break; } | |
| Err(_) => { tracing::warn!("router websocket write timed out"); break; } | |
| } | |
| } | |
| None => break, // all senders dropped → close | |
| }, | |
| // Inbound: decode and forward as an event. | |
| item = stream.next() => match item { | |
| Some(Ok(Message::Text(t))) => { | |
| match serde_json::from_str::<ServerMsg>(t.as_str()) { | |
| Ok(m) => { | |
| if ev_tx.send(FillerEvent::from(m)).is_err() { | |
| break; // receiver dropped | |
| } | |
| } | |
| Err(e) => tracing::warn!(error = %e, "drop undecodable server frame"), | |
| } | |
| } | |
| Some(Ok(Message::Close(_))) | Some(Err(_)) | None => break, | |
| Some(Ok(_)) => {} // ignore binary/ping/pong | |
| }, | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/filler-sdk/src/client.rs` around lines 218 - 249, The outbound write
path in the client pump can stall the whole tokio::select! loop because
sink.send() is awaited without a bound, which blocks stream.next() and violates
the non-blocking contract. Update the loop in the client message pump to apply a
write timeout around the sink.send(Message::Text(...)) call, and on timeout
treat it as a send failure path that breaks or otherwise closes the pump
cleanly. Keep the fix localized to the pump loop and preserve the existing
handling of serialization errors, inbound decode, and FillerEvent forwarding.
Comment on lines
+56
to
+73
| impl PswapTerms { | ||
| /// Parse the PSWAP terms out of a decoded note. Errors if the note is not a | ||
| /// well-formed PSWAP note. | ||
| pub fn from_note(note: &Note) -> Result<Self> { | ||
| let pswap = | ||
| PswapNote::try_from(note).map_err(|e| anyhow!("not a PSWAP note: {e}"))?; | ||
| let offered = pswap.offered_asset(); | ||
| let storage = pswap.storage(); | ||
| let requested = storage.requested_asset(); | ||
| Ok(Self { | ||
| offered_faucet: offered.faucet_id(), | ||
| offered_amount: offered.amount().into(), | ||
| requested_faucet: requested.faucet_id(), | ||
| requested_amount: requested.amount().into(), | ||
| creator: storage.creator_account_id(), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the PswapNote definition and its accessor return types.
fd -ti pswap --exec rg -nP -C2 'fn\s+(offered_asset|requested_asset|faucet_id|amount|creator_account_id|create_args|try_from)\s*\(' {} \;
# Cargo manifest for the pinned miden-standards / miden-protocol versions.
fd -t f 'Cargo.toml' crates/filler-sdk --exec rg -nP 'miden-(standards|protocol)' {} \;Repository: inicio-labs/solver
Length of output: 512
Update asset amount retrieval to use as_u64().
FungibleAsset::amount() returns an AssetAmount wrapper, not a primitive u64. Consequently, the .into() conversion is not a no-op; it is unsafe or non-idiomatic. The correct API method to extract the value is .as_u64().
Diff
let offered = pswap.offered_asset();
let storage = pswap.storage();
let requested = storage.requested_asset();
Ok(Self {
offered_faucet: offered.faucet_id(),
- offered_amount: offered.amount().into(),
+ offered_amount: offered.amount().as_u64(),
requested_faucet: requested.faucet_id(),
- requested_amount: requested.amount().into(),
+ requested_amount: requested.amount().as_u64(),
creator: storage.creator_account_id(),
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| impl PswapTerms { | |
| /// Parse the PSWAP terms out of a decoded note. Errors if the note is not a | |
| /// well-formed PSWAP note. | |
| pub fn from_note(note: &Note) -> Result<Self> { | |
| let pswap = | |
| PswapNote::try_from(note).map_err(|e| anyhow!("not a PSWAP note: {e}"))?; | |
| let offered = pswap.offered_asset(); | |
| let storage = pswap.storage(); | |
| let requested = storage.requested_asset(); | |
| Ok(Self { | |
| offered_faucet: offered.faucet_id(), | |
| offered_amount: offered.amount().into(), | |
| requested_faucet: requested.faucet_id(), | |
| requested_amount: requested.amount().into(), | |
| creator: storage.creator_account_id(), | |
| }) | |
| } | |
| } | |
| impl PswapTerms { | |
| /// Parse the PSWAP terms out of a decoded note. Errors if the note is not a | |
| /// well-formed PSWAP note. | |
| pub fn from_note(note: &Note) -> Result<Self> { | |
| let pswap = | |
| PswapNote::try_from(note).map_err(|e| anyhow!("not a PSWAP note: {e}"))?; | |
| let offered = pswap.offered_asset(); | |
| let storage = pswap.storage(); | |
| let requested = storage.requested_asset(); | |
| Ok(Self { | |
| offered_faucet: offered.faucet_id(), | |
| offered_amount: offered.amount().as_u64(), | |
| requested_faucet: requested.faucet_id(), | |
| requested_amount: requested.amount().as_u64(), | |
| creator: storage.creator_account_id(), | |
| }) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/filler-sdk/src/consume.rs` around lines 56 - 73, In
PswapTerms::from_note, the offered and requested amount fields are currently
derived from FungibleAsset::amount() using a generic conversion, but amount()
returns an AssetAmount wrapper, so update the extraction to use the explicit
as_u64() accessor instead. Keep the rest of the parsing logic in
PswapNote::try_from, offered_asset(), and storage().requested_asset() unchanged,
and make sure both offered_amount and requested_amount are populated from the
wrapper’s u64 value via as_u64().
Add an explicit numbered "integration in 5 steps" and a Mermaid sequence diagram of the RFQ conversation (connect/auth → subscribe/quote → matcher select+park → handover → on-chain consume → fill observed → drop) to both the integration guide and the SDK README.
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
The solver never sends ServerMsg::Withdrawn, so FillerEvent::Withdrawn was a dead API surface implying a guarantee that doesn't exist — a DEX is never actually told when a handed-over note is pulled. Drop it from the protocol, the client event enum, and the docs. Can be re-added if/when it is wired to emit (e.g. on reactivation, before re-routing). Also syncs Cargo.lock to this branch's manifests (the SDK-only branch has no solver ws/futures-util deps).
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.
What
Adds
pswap-filler-sdk— the crate external DEXes ("fillers") depend on tointegrate with the solver's RFQ router. A DEX takes on only this crate, not the
solver /
miden-client/diesel/axum/ internal modules.Design
Full reasoning in
docs/adr/0002-filler-sdk.md. In short:protocolmodule). The solver dependson the SDK for it one-way, so the two sides can't drift.
FillerClient::connect(url, token)→subscribe/quote/next_event(
FillerEvent), with a background pump task so send/receive never block each other.consumefeature (opt-in) adds on-chain helpers (decode_note,PswapTerms,consume_args) that pull onlymiden-protocol+miden-standards— nevermiden-client. The DEX runs the consume transaction with its own client/keystore/gas.Testing
--features consume); no warningseither mode.
cargo tree: the default build has nomiden / diesel / axum / tonic;
consumeadds only the miden-protocol stack.Docs
docs/filler-integration.md— DEX-facing integration guide (protocol reference,quoting/decimal semantics, a complete reference filler, FAQ).
docs/adr/0002-filler-sdk.md— the ADR.Base branch for the stacked external-liquidity-routing PR.
Summary by CodeRabbit