chore(minibf): Implement /addresses/address#925
Conversation
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new /addresses/{address} endpoint with richer address parsing (Payment, Shelley, Byron), key-based UTXO lookups, address-to-model mapping, and consolidated error response construction via a private ErrorBody type. Includes tests and small refactors in tests/pagination. Changes
Sequence DiagramsequenceDiagram
actor Client
participant Router as "Router\n(minibf)"
participant Parser as "AddressParser"
participant UTXO as "UTXOResolver"
participant DB as "Database"
participant Validator as "ChainValidator"
participant Builder as "ResponseBuilder"
Client->>Router: GET /addresses/{address}
Router->>Parser: parse_address(address_str)
Parser-->>Router: ParsedAddress (Payment/Shelley/Byron)
Router->>UTXO: refs_for_parsed_address(parsed)
UTXO->>DB: Query UTXO refs by key
DB-->>UTXO: UTXO refs
UTXO-->>Router: refs
Router->>Validator: is_address_in_chain(address)
Validator->>DB: Stream blocks / scan for address
DB-->>Validator: found / not_found
Validator-->>Router: bool
alt refs found or in chain
Router->>Builder: amount_for_refs(refs)
Builder->>DB: Fetch UTXO assets
DB-->>Builder: assets
Builder-->>Router: AddressContent
Router-->>Client: 200 JSON(AddressContent)
else not found
Router-->>Client: 404 Not Found (ErrorBody)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/minibf/src/routes/addresses.rs`:
- Around line 247-249: The current code calls is_address_in_chain(&domain,
&address).await when refs.is_empty(), which may stream the full chain per
request; replace that full-chain scan with an indexed or limited check: either
call a fast index lookup (e.g., implement and use
address_exists_in_index(domain, address)) or modify is_address_in_chain to
accept a max_scan_limit and use that (e.g.,
is_address_in_chain_with_limit(domain, address, max_slots)) and return NOT_FOUND
when the limit is exceeded; update the call site to use the index or the bounded
scan and ensure refs, domain, and address flow into the new check instead of
unbounded full-chain scanning.
- Around line 73-78: The current check uses
address.starts_with("addr_vkh")/("script") which is too permissive; instead call
bech32::decode(address).map_err(|_| Error::InvalidAddress)? to get (hrp, addr)
and then validate hrp exactly equals "addr_vkh" or "script" (e.g., if hrp !=
"addr_vkh" && hrp != "script" return Err(Error::InvalidAddress)), then return
Ok(ParsedAddress::Payment { key: addr, script: hrp.as_str() == "script" });
remove the starts_with branch and rely on exact HRP comparison around
bech32::decode/ParsedAddress::Payment/Error::InvalidAddress.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
crates/minibf/src/error.rscrates/minibf/src/lib.rscrates/minibf/src/pagination.rscrates/minibf/src/routes/addresses.rscrates/minibf/src/routes/pools.rscrates/minibf/src/test_support.rs
| if address.starts_with("addr_vkh") || address.starts_with("script") { | ||
| let (_, addr) = bech32::decode(address).map_err(|_| Error::InvalidAddress)?; | ||
| return Ok(ParsedAddress::Payment(addr)); | ||
| let (hrp, addr) = bech32::decode(address).map_err(|_| Error::InvalidAddress)?; | ||
| return Ok(ParsedAddress::Payment { | ||
| key: addr, | ||
| script: hrp.as_str() == "script", | ||
| }); |
There was a problem hiding this comment.
Validate payment-credential HRP exactly (not by prefix).
Line 73 currently accepts any bech32 string starting with addr_vkh/script, which can treat unsupported HRPs as valid and return 404 instead of 400.
🔧 Suggested fix
- if address.starts_with("addr_vkh") || address.starts_with("script") {
- let (hrp, addr) = bech32::decode(address).map_err(|_| Error::InvalidAddress)?;
- return Ok(ParsedAddress::Payment {
- key: addr,
- script: hrp.as_str() == "script",
- });
- }
+ if let Ok((hrp, key)) = bech32::decode(address) {
+ match hrp.as_str() {
+ "addr_vkh" | "script" if key.len() == 28 => {
+ return Ok(ParsedAddress::Payment {
+ script: hrp.as_str() == "script",
+ key,
+ });
+ }
+ "addr_vkh" | "script" => return Err(Error::InvalidAddress),
+ _ => {}
+ }
+ }📝 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.
| if address.starts_with("addr_vkh") || address.starts_with("script") { | |
| let (_, addr) = bech32::decode(address).map_err(|_| Error::InvalidAddress)?; | |
| return Ok(ParsedAddress::Payment(addr)); | |
| let (hrp, addr) = bech32::decode(address).map_err(|_| Error::InvalidAddress)?; | |
| return Ok(ParsedAddress::Payment { | |
| key: addr, | |
| script: hrp.as_str() == "script", | |
| }); | |
| if let Ok((hrp, key)) = bech32::decode(address) { | |
| match hrp.as_str() { | |
| "addr_vkh" | "script" if key.len() == 28 => { | |
| return Ok(ParsedAddress::Payment { | |
| script: hrp.as_str() == "script", | |
| key, | |
| }); | |
| } | |
| "addr_vkh" | "script" => return Err(Error::InvalidAddress), | |
| _ => {} | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/minibf/src/routes/addresses.rs` around lines 73 - 78, The current
check uses address.starts_with("addr_vkh")/("script") which is too permissive;
instead call bech32::decode(address).map_err(|_| Error::InvalidAddress)? to get
(hrp, addr) and then validate hrp exactly equals "addr_vkh" or "script" (e.g.,
if hrp != "addr_vkh" && hrp != "script" return Err(Error::InvalidAddress)), then
return Ok(ParsedAddress::Payment { key: addr, script: hrp.as_str() == "script"
}); remove the starts_with branch and rely on exact HRP comparison around
bech32::decode/ParsedAddress::Payment/Error::InvalidAddress.
| if refs.is_empty() && !is_address_in_chain(&domain, &address).await? { | ||
| return Err(StatusCode::NOT_FOUND.into()); | ||
| } |
There was a problem hiding this comment.
/addresses/{address} miss-path can trigger full-chain scans per request.
Line 247 calls is_address_in_chain, which streams from slot 0 to tip on empty refs. For unknown addresses, this is O(chain length) work per request and can become a reliability hotspot under abuse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/minibf/src/routes/addresses.rs` around lines 247 - 249, The current
code calls is_address_in_chain(&domain, &address).await when refs.is_empty(),
which may stream the full chain per request; replace that full-chain scan with
an indexed or limited check: either call a fast index lookup (e.g., implement
and use address_exists_in_index(domain, address)) or modify is_address_in_chain
to accept a max_scan_limit and use that (e.g.,
is_address_in_chain_with_limit(domain, address, max_slots)) and return NOT_FOUND
when the limit is exceeded; update the call site to use the index or the bounded
scan and ensure refs, domain, and address flow into the new check instead of
unbounded full-chain scanning.
Summary by CodeRabbit
New Features
Improvements
Tests