refactor: display all makers without filters - #8
Merged
Merged
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR refactors the market data API from offers-centric to makers-centric. The system now tracks all known makers (good, unresponsive, bad) with their connection state, protocol, and optional embedded offers, replacing the previous filtered view of only good makers with active offers. ChangesMakers-Centric API Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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: 3
🤖 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 `@tests/http.rs`:
- Around line 101-141: Add focused tests in tests/http.rs that call the makers
endpoint with the state filter (use
guard.agent.get(&guard.url("/api/makers?state=good")), "/api/makers?state=bad",
and "/api/makers?state=unresponsive") and assert the returned array length and
contents match the expected single-kind contract: for "good" assert the returned
maker has state.kind=="good" and contains the offer/protocol fields (e.g.,
base_fee and fidelity_bond present), for "unresponsive" assert
state.kind=="unresponsive", state.retries==3 and that offer and protocol are
null, and for "bad" assert state.kind=="bad" and offer is null; reuse the
existing pattern (resp.into_json(), as_array(), indexing into arr) so tests
mirror the earlier checks around the GET /api/makers block.
In `@web/src/App.jsx`:
- Around line 51-54: The footer's lastUpdated is never cleared when makers
become empty; update the logic around data handling in App.jsx so that if
data.length === 0 you call setLastUpdated(null) (or undefined) to clear the
stale timestamp, otherwise keep the existing branch that computes latest with
Math.max and calls setLastUpdated(new Date(latest)); modify the block around the
existing setLastUpdated usage (referencing setLastUpdated and the data
map/timestamp calculation) to add the else branch that clears the state.
- Around line 148-150: The row key currently switches between bond-based
(`${bond.outpoint.txid}:${bond.outpoint.vout}`) and `m.address`, causing
remounts when bond presence changes; make the key stable by always using the
maker identifier (use `m.address`) as the primary key (optionally append bond
details but do not replace the base key) so the row keeps the same key whether
`bond` is present or not.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b431f701-3489-4871-a07c-3c39960b484b
📒 Files selected for processing (7)
.github/workflows/docker-build.ymlsrc/server.rssrc/state.rssrc/sync.rstests/e2e.rstests/http.rsweb/src/App.jsx
Comment on lines
101
to
141
| let resp = guard | ||
| .agent | ||
| .get(&guard.url("/api/offers")) | ||
| .get(&guard.url("/api/makers")) | ||
| .call() | ||
| .expect("GET /api/offers"); | ||
| .expect("GET /api/makers"); | ||
| assert_eq!(resp.status(), 200); | ||
| let offers: Value = resp.into_json().expect("offers JSON"); | ||
| let arr = offers.as_array().expect("array"); | ||
| assert_eq!(arr.len(), 1); | ||
| let makers: Value = resp.into_json().expect("makers JSON"); | ||
| let arr = makers.as_array().expect("array"); | ||
| assert_eq!(arr.len(), 3, "bad and unresponsive makers must be returned"); | ||
|
|
||
| // Good maker | ||
| let good = &arr[0]; | ||
| assert_eq!(good["address"], "127.0.0.1:6102"); | ||
| assert_eq!(good["state"]["kind"], "good"); | ||
| assert_eq!(good["protocol"], "taproot"); | ||
| assert_eq!(good["offer"]["base_fee"], 1000); | ||
| assert_eq!(good["offer"]["fidelity_bond"]["amount"], 5_000_000); | ||
| assert_eq!(good["offer"]["fidelity_bond"]["outpoint"]["txid"], "deadbeef"); | ||
|
|
||
| // Unresponsive maker — kind=unresponsive with a retries count. | ||
| let unresp = &arr[1]; | ||
| assert_eq!(unresp["state"]["kind"], "unresponsive"); | ||
| assert_eq!(unresp["state"]["retries"], 3); | ||
| assert!(unresp["offer"].is_null(), "offer must be null for unresponsive maker"); | ||
| assert!(unresp["protocol"].is_null()); | ||
|
|
||
| let o = &arr[0]; | ||
| assert_eq!(o["address"], "test.onion:6102"); | ||
| assert_eq!(o["base_fee"], 1000); | ||
| assert_eq!(o["fidelity_bond"]["amount"], 5_000_000); | ||
| assert_eq!(o["fidelity_bond"]["outpoint"]["txid"], "deadbeef"); | ||
| assert_eq!(o["fidelity_bond"]["outpoint"]["vout"], 0); | ||
| // Bad maker — same shape, kind=bad. | ||
| let bad = &arr[2]; | ||
| assert_eq!(bad["state"]["kind"], "bad"); | ||
| assert!(bad["offer"].is_null()); | ||
|
|
||
| let resp = guard | ||
| .agent | ||
| .get(&guard.url("/api/health")) | ||
| .call() | ||
| .expect("GET /api/health"); | ||
| let body: Value = resp.into_json().expect("health JSON"); | ||
| assert_eq!(body["offer_count"], 1); | ||
| assert_eq!(body["maker_count"], 3); | ||
| assert_eq!(body["with_offer"], 1); | ||
| assert_eq!(body["last_sync"], 1_700_000_000); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add direct coverage for /api/makers?state=... filter behavior.
Current smoke tests validate shape/counts but not the new state-filter contract; add focused assertions for good, bad, and unresponsive queries.
Suggested test addition
+ // Filtered query: good only
+ let resp = guard
+ .agent
+ .get(&guard.url("/api/makers?state=good"))
+ .call()
+ .expect("GET /api/makers?state=good");
+ let only_good: Value = resp.into_json().expect("good makers JSON");
+ assert_eq!(only_good.as_array().expect("array").len(), 1);
+ assert_eq!(only_good[0]["state"]["kind"], "good");📝 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
| let resp = guard | |
| .agent | |
| .get(&guard.url("/api/offers")) | |
| .get(&guard.url("/api/makers")) | |
| .call() | |
| .expect("GET /api/offers"); | |
| .expect("GET /api/makers"); | |
| assert_eq!(resp.status(), 200); | |
| let offers: Value = resp.into_json().expect("offers JSON"); | |
| let arr = offers.as_array().expect("array"); | |
| assert_eq!(arr.len(), 1); | |
| let makers: Value = resp.into_json().expect("makers JSON"); | |
| let arr = makers.as_array().expect("array"); | |
| assert_eq!(arr.len(), 3, "bad and unresponsive makers must be returned"); | |
| // Good maker | |
| let good = &arr[0]; | |
| assert_eq!(good["address"], "127.0.0.1:6102"); | |
| assert_eq!(good["state"]["kind"], "good"); | |
| assert_eq!(good["protocol"], "taproot"); | |
| assert_eq!(good["offer"]["base_fee"], 1000); | |
| assert_eq!(good["offer"]["fidelity_bond"]["amount"], 5_000_000); | |
| assert_eq!(good["offer"]["fidelity_bond"]["outpoint"]["txid"], "deadbeef"); | |
| // Unresponsive maker — kind=unresponsive with a retries count. | |
| let unresp = &arr[1]; | |
| assert_eq!(unresp["state"]["kind"], "unresponsive"); | |
| assert_eq!(unresp["state"]["retries"], 3); | |
| assert!(unresp["offer"].is_null(), "offer must be null for unresponsive maker"); | |
| assert!(unresp["protocol"].is_null()); | |
| let o = &arr[0]; | |
| assert_eq!(o["address"], "test.onion:6102"); | |
| assert_eq!(o["base_fee"], 1000); | |
| assert_eq!(o["fidelity_bond"]["amount"], 5_000_000); | |
| assert_eq!(o["fidelity_bond"]["outpoint"]["txid"], "deadbeef"); | |
| assert_eq!(o["fidelity_bond"]["outpoint"]["vout"], 0); | |
| // Bad maker — same shape, kind=bad. | |
| let bad = &arr[2]; | |
| assert_eq!(bad["state"]["kind"], "bad"); | |
| assert!(bad["offer"].is_null()); | |
| let resp = guard | |
| .agent | |
| .get(&guard.url("/api/health")) | |
| .call() | |
| .expect("GET /api/health"); | |
| let body: Value = resp.into_json().expect("health JSON"); | |
| assert_eq!(body["offer_count"], 1); | |
| assert_eq!(body["maker_count"], 3); | |
| assert_eq!(body["with_offer"], 1); | |
| assert_eq!(body["last_sync"], 1_700_000_000); | |
| } | |
| let resp = guard | |
| .agent | |
| .get(&guard.url("/api/makers")) | |
| .call() | |
| .expect("GET /api/makers"); | |
| assert_eq!(resp.status(), 200); | |
| let makers: Value = resp.into_json().expect("makers JSON"); | |
| let arr = makers.as_array().expect("array"); | |
| assert_eq!(arr.len(), 3, "bad and unresponsive makers must be returned"); | |
| // Good maker | |
| let good = &arr[0]; | |
| assert_eq!(good["address"], "127.0.0.1:6102"); | |
| assert_eq!(good["state"]["kind"], "good"); | |
| assert_eq!(good["protocol"], "taproot"); | |
| assert_eq!(good["offer"]["base_fee"], 1000); | |
| assert_eq!(good["offer"]["fidelity_bond"]["amount"], 5_000_000); | |
| assert_eq!(good["offer"]["fidelity_bond"]["outpoint"]["txid"], "deadbeef"); | |
| // Unresponsive maker — kind=unresponsive with a retries count. | |
| let unresp = &arr[1]; | |
| assert_eq!(unresp["state"]["kind"], "unresponsive"); | |
| assert_eq!(unresp["state"]["retries"], 3); | |
| assert!(unresp["offer"].is_null(), "offer must be null for unresponsive maker"); | |
| assert!(unresp["protocol"].is_null()); | |
| // Bad maker — same shape, kind=bad. | |
| let bad = &arr[2]; | |
| assert_eq!(bad["state"]["kind"], "bad"); | |
| assert!(bad["offer"].is_null()); | |
| // Filtered query: good only | |
| let resp = guard | |
| .agent | |
| .get(&guard.url("/api/makers?state=good")) | |
| .call() | |
| .expect("GET /api/makers?state=good"); | |
| let only_good: Value = resp.into_json().expect("good makers JSON"); | |
| assert_eq!(only_good.as_array().expect("array").len(), 1); | |
| assert_eq!(only_good[0]["state"]["kind"], "good"); | |
| let resp = guard | |
| .agent | |
| .get(&guard.url("/api/health")) | |
| .call() | |
| .expect("GET /api/health"); | |
| let body: Value = resp.into_json().expect("health JSON"); | |
| assert_eq!(body["maker_count"], 3); | |
| assert_eq!(body["with_offer"], 1); | |
| assert_eq!(body["last_sync"], 1_700_000_000); | |
| } |
🤖 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 `@tests/http.rs` around lines 101 - 141, Add focused tests in tests/http.rs
that call the makers endpoint with the state filter (use
guard.agent.get(&guard.url("/api/makers?state=good")), "/api/makers?state=bad",
and "/api/makers?state=unresponsive") and assert the returned array length and
contents match the expected single-kind contract: for "good" assert the returned
maker has state.kind=="good" and contains the offer/protocol fields (e.g.,
base_fee and fidelity_bond present), for "unresponsive" assert
state.kind=="unresponsive", state.retries==3 and that offer and protocol are
null, and for "bad" assert state.kind=="bad" and offer is null; reuse the
existing pattern (resp.into_json(), as_array(), indexing into arr) so tests
mirror the earlier checks around the GET /api/makers block.
Comment on lines
51
to
54
| if (data.length > 0) { | ||
| const latest = Math.max(...data.map((o) => o.timestamp * 1000)); | ||
| const latest = Math.max(...data.map((m) => m.timestamp * 1000)); | ||
| setLastUpdated(new Date(latest)); | ||
| } |
There was a problem hiding this comment.
Clear stale lastUpdated when no makers are returned.
lastUpdated is only set for non-empty data. If the list later becomes empty, the footer can still show an old update time.
Suggested fix
if (data.length > 0) {
const latest = Math.max(...data.map((m) => m.timestamp * 1000));
setLastUpdated(new Date(latest));
+ } else {
+ setLastUpdated(null);
}📝 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
| if (data.length > 0) { | |
| const latest = Math.max(...data.map((o) => o.timestamp * 1000)); | |
| const latest = Math.max(...data.map((m) => m.timestamp * 1000)); | |
| setLastUpdated(new Date(latest)); | |
| } | |
| if (data.length > 0) { | |
| const latest = Math.max(...data.map((m) => m.timestamp * 1000)); | |
| setLastUpdated(new Date(latest)); | |
| } else { | |
| setLastUpdated(null); | |
| } |
🤖 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 `@web/src/App.jsx` around lines 51 - 54, The footer's lastUpdated is never
cleared when makers become empty; update the logic around data handling in
App.jsx so that if data.length === 0 you call setLastUpdated(null) (or
undefined) to clear the stale timestamp, otherwise keep the existing branch that
computes latest with Math.max and calls setLastUpdated(new Date(latest)); modify
the block around the existing setLastUpdated usage (referencing setLastUpdated
and the data map/timestamp calculation) to add the else branch that clears the
state.
Comment on lines
+148
to
+150
| const key = bond | ||
| ? `${bond.outpoint.txid}:${bond.outpoint.vout}` | ||
| : m.address; |
There was a problem hiding this comment.
Use a stable key per maker row.
Lines 149-150 switch key source based on optional bond data. If a maker toggles between having an offer and not having one, the key changes and the row remounts unnecessarily.
Suggested fix
- const key = bond
- ? `${bond.outpoint.txid}:${bond.outpoint.vout}`
- : m.address;
+ const key = m.address;📝 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
| const key = bond | |
| ? `${bond.outpoint.txid}:${bond.outpoint.vout}` | |
| : m.address; | |
| const key = m.address; |
🤖 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 `@web/src/App.jsx` around lines 148 - 150, The row key currently switches
between bond-based (`${bond.outpoint.txid}:${bond.outpoint.vout}`) and
`m.address`, causing remounts when bond presence changes; make the key stable by
always using the maker identifier (use `m.address`) as the primary key
(optionally append bond details but do not replace the base key) so the row
keeps the same key whether `bond` is present or not.
hulxv
force-pushed
the
refactor/display-all-makers
branch
from
June 3, 2026 11:33
32529de to
47a20c2
Compare
Merged
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 by CodeRabbit
Release Notes
New Features
/api/makersreturning comprehensive maker information including connection state and protocol details.maker_count,with_offer) instead of offer counts.Tests