feat(minibf): implement /accounts/{stake_address}/transactions#1127
Open
michalrus wants to merge 2 commits into
Open
feat(minibf): implement /accounts/{stake_address}/transactions#1127michalrus wants to merge 2 commits into
/accounts/{stake_address}/transactions#1127michalrus wants to merge 2 commits into
Conversation
This adds the account transactions endpoint, listing transactions
associated with a stake address. It scans the `stake` archive index and
emits one entry per distinct `(transaction, address)` pair the account
participates in, through either produced outputs or resolved consumed
inputs, mirroring the `/addresses/{address}/transactions` endpoint.
The other six account endpoints now also accept CIP-19 stake credential
bech32 forms:(`stake_vk`, `stake_vkh`, `script`) in addition to full
stake addresses, resolving them against the node's network. This change
was required to make `blockfrost-tests` pass on the new endpoint, and it
only makes sense to include it for the already existing ones.
|
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 selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe accounts API adds CIP-19 stake credential resolution, propagates network-aware parsing across stake routes, and exposes a paginated transactions endpoint that matches produced and consumed stake-related addresses. ChangesStake account API
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant by_stake_transactions
participant IndexStore
Client->>by_stake_transactions: request account transactions
by_stake_transactions->>IndexStore: verify account and scan stake-indexed blocks
IndexStore-->>by_stake_transactions: blocks and transaction data
by_stake_transactions-->>Client: paginated transaction results
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
Pull request overview
Adds Blockfrost-compatible /accounts/{stake_address}/transactions support to the minibf service, and broadens account stake identifier parsing to accept CIP-19 credential bech32 forms resolved against the node network.
Changes:
- Implement
/accounts/{stake_address}/transactionsby scanning thestakearchive index and extracting distinct(tx, address)participations from both produced outputs and resolved consumed inputs. - Extend existing account routes to accept CIP-19 credential forms (
stake_vk,stake_vkh,script) in addition to canonical stake addresses. - Add unit/integration tests covering CIP-19 parsing and the new transactions endpoint behavior (happy path, pagination, ordering, and error cases).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/minibf/src/routes/accounts.rs | Adds CIP-19 stake credential parsing and implements the new account transactions endpoint (plus tests). |
| crates/minibf/src/lib.rs | Registers the new /accounts/{stake_address}/transactions HTTP route. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+910
to
+914
| async fn account_addresses_in_tx<D>( | ||
| domain: &Facade<D>, | ||
| account: &[u8], | ||
| tx: &MultiEraTx<'_>, | ||
| ) -> Result<BTreeSet<String>, StatusCode> |
Contributor
Author
There was a problem hiding this comment.
Hmmm, it could look like this (below).
The tests still pass.
@scarmuega, but where to put this shared helper, fn tx_touched_addresses? Maybe crates/minibf/src/routes/mod.rs is a little too shared and disorderly?
diff --git a/crates/minibf/src/routes/accounts.rs b/crates/minibf/src/routes/accounts.rs
index 13831d5a..10626df3 100644
--- a/crates/minibf/src/routes/accounts.rs
+++ b/crates/minibf/src/routes/accounts.rs
@@ -22,7 +22,7 @@ use dolos_cardano::{
pallas_extras, ChainSummary, FixedNamespace, LeaderRewardLog, MemberRewardLog,
PoolDepositRefundLog,
};
-use dolos_core::{ArchiveStore as _, Domain, EntityKey, EraCbor, LogKey, TemporalKey};
+use dolos_core::{ArchiveStore as _, Domain, EntityKey, LogKey, TemporalKey};
use futures_util::StreamExt;
use pallas::{
codec::minicbor,
@@ -915,45 +915,13 @@ async fn account_addresses_in_tx<D>(
where
D: Domain + Clone + Send + Sync + 'static,
{
- let mut addresses = BTreeSet::new();
+ let touched = crate::routes::tx_touched_addresses(domain, tx).await?;
- for (_, output) in tx.produces() {
- let candidate = output
- .address()
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-
- if address_belongs_to_account(&candidate, account) {
- addresses.insert(candidate.to_string());
- }
- }
-
- for input in tx.consumes() {
- if let Some(EraCbor(era, cbor)) = domain
- .query()
- .tx_cbor(input.hash().as_slice().to_vec())
- .await
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?
- {
- let parsed = MultiEraTx::decode_for_era(
- era.try_into()
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?,
- &cbor,
- )
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-
- if let Some(output) = parsed.produces_at(input.index() as usize) {
- let candidate = output
- .address()
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-
- if address_belongs_to_account(&candidate, account) {
- addresses.insert(candidate.to_string());
- }
- }
- }
- }
-
- Ok(addresses)
+ Ok(touched
+ .into_iter()
+ .filter(|candidate| address_belongs_to_account(candidate, account))
+ .map(|candidate| candidate.to_string())
+ .collect())
}
async fn find_account_txs_in_block<D>(
diff --git a/crates/minibf/src/routes/addresses.rs b/crates/minibf/src/routes/addresses.rs
index d8623789..5a72243b 100644
--- a/crates/minibf/src/routes/addresses.rs
+++ b/crates/minibf/src/routes/addresses.rs
@@ -22,7 +22,7 @@ use dolos_cardano::{
indexes::{AsyncCardanoQueryExt, CardanoIndexExt, SlotOrder},
pallas_extras, ChainSummary,
};
-use dolos_core::{BlockBody, BlockSlot, Domain, EraCbor, StateStore as _, TxoRef};
+use dolos_core::{BlockBody, BlockSlot, Domain, StateStore as _, TxoRef};
use crate::{
error::Error,
@@ -386,42 +386,10 @@ async fn has_address<D>(
where
D: Domain + Clone + Send + Sync + 'static,
{
- for (_, output) in tx.produces() {
- let candidate = output
- .address()
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-
- if address_matches(address, &candidate) {
- return Ok(true);
- }
- }
-
- for input in tx.consumes() {
- if let Some(EraCbor(era, cbor)) = domain
- .query()
- .tx_cbor(input.hash().as_slice().to_vec())
- .await
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?
- {
- let parsed = MultiEraTx::decode_for_era(
- era.try_into()
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?,
- &cbor,
- )
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
- if let Some(output) = parsed.produces_at(input.index() as usize) {
- let candidate = output
- .address()
- .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
-
- if address_matches(address, &candidate) {
- return Ok(true);
- }
- }
- }
- }
-
- Ok(false)
+ let touched = crate::routes::tx_touched_addresses(domain, tx).await?;
+ Ok(touched
+ .iter()
+ .any(|candidate| address_matches(address, candidate)))
}
async fn find_txs<D>(
diff --git a/crates/minibf/src/routes/mod.rs b/crates/minibf/src/routes/mod.rs
index a062670a..380fd259 100644
--- a/crates/minibf/src/routes/mod.rs
+++ b/crates/minibf/src/routes/mod.rs
@@ -18,11 +18,60 @@ pub mod utxos;
use std::env;
use axum::{extract::State, http::StatusCode, Json};
-use dolos_core::Domain;
+use dolos_core::{Domain, EraCbor};
+use pallas::ledger::{addresses::Address, traverse::MultiEraTx};
use serde::{Deserialize, Serialize};
use crate::{Facade, MinibfConfig};
+/// Every address a transaction touches: the addresses of its produced
+/// outputs plus the resolved outputs of its consumed inputs.
+pub(crate) async fn tx_touched_addresses<D>(
+ domain: &Facade<D>,
+ tx: &MultiEraTx<'_>,
+) -> Result<Vec<Address>, StatusCode>
+where
+ D: Domain + Clone + Send + Sync + 'static,
+{
+ let mut addresses = Vec::new();
+
+ for (_, output) in tx.produces() {
+ addresses.push(
+ output
+ .address()
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?,
+ );
+ }
+
+ for input in tx.consumes() {
+ let Some(EraCbor(era, cbor)) = domain
+ .query()
+ .tx_cbor(input.hash().as_slice().to_vec())
+ .await
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?
+ else {
+ continue;
+ };
+
+ let parsed = MultiEraTx::decode_for_era(
+ era.try_into()
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?,
+ &cbor,
+ )
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?;
+
+ if let Some(output) = parsed.produces_at(input.index() as usize) {
+ addresses.push(
+ output
+ .address()
+ .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?,
+ );
+ }
+ }
+
+ Ok(addresses)
+}
+
#[derive(Debug, Serialize, Deserialize)]
pub struct RootResponse {
url: String,
Comment on lines
+931
to
+935
| if let Some(EraCbor(era, cbor)) = domain | ||
| .query() | ||
| .tx_cbor(input.hash().as_slice().to_vec()) | ||
| .await | ||
| .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)? |
Contributor
Author
There was a problem hiding this comment.
Good observation, but account_addresses_in_tx deliberately mirrors addresses/transactions, which uses the identical tx_cbor-per-input pattern.
@scarmuega WDYT? I think we'd prefer to fix that as a dedicated follow-up that optimizes both endpoints together rather than diverging them here?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/minibf/src/routes/accounts.rs (2)
900-908: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
address_belongs_to_accountinby_stake_addresses.The same match already exists inline at lines 269-277; switching that call site to this helper keeps one definition of the matching rule. Note
to_vec()is also recomputed per candidate there — passing the precomputedaccountbytes avoids that.🤖 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/minibf/src/routes/accounts.rs` around lines 900 - 908, The inline address-matching logic in by_stake_addresses should be replaced with calls to address_belongs_to_account. Pass the precomputed account bytes to the helper for each candidate so the matching rule has one definition and account conversion is not repeated.
930-954: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winGroup consumed inputs by parent tx hash.
transactions<D>()fetches and decodes the same parent transaction for every spent input, so a transaction spending multiple inputs from one parent can execute repeated identicaltx_cborreads and CBOR decodes sequentially. Grouptx.consumes()byinput.hash()and resolve each parent once; this also keeps the implementation aligned with the mirrored address-transaction flow.🤖 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/minibf/src/routes/accounts.rs` around lines 930 - 954, Update the consumed-input handling around the tx.consumes() loop to group inputs by input.hash() before querying parent transactions. Fetch and decode each unique parent transaction once, then process all grouped input indices with the existing produces_at and address_belongs_to_account logic, preserving current error handling and address collection behavior.
🤖 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.
Nitpick comments:
In `@crates/minibf/src/routes/accounts.rs`:
- Around line 900-908: The inline address-matching logic in by_stake_addresses
should be replaced with calls to address_belongs_to_account. Pass the
precomputed account bytes to the helper for each candidate so the matching rule
has one definition and account conversion is not repeated.
- Around line 930-954: Update the consumed-input handling around the
tx.consumes() loop to group inputs by input.hash() before querying parent
transactions. Fetch and decode each unique parent transaction once, then process
all grouped input indices with the existing produces_at and
address_belongs_to_account logic, preserving current error handling and address
collection behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5b6e783f-d7ee-49c7-b540-112c2a8ef78f
📒 Files selected for processing (2)
crates/minibf/src/lib.rscrates/minibf/src/routes/accounts.rs
Compute `tx_hash`, `block_height`, and `block_time` once per transaction in the account transactions scan instead of recomputing them for every address, cloning the precomputed hash string per entry.
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
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.
Resolves #1074.
Implementation
This adds the account transactions endpoint, listing transactions associated with a stake address. It scans the
stakearchive index and emits one entry per distinct(transaction, address)pair the account participates in, through either produced outputs or resolved consumed inputs, mirroring the/addresses/{address}/transactionsendpoint.The other six account endpoints now also accept CIP-19 stake credential bech32 forms:(
stake_vk,stake_vkh,script) in addition to full stake addresses, resolving them against the node's network. This change was required to makeblockfrost-testspass on the new endpoint, and it only makes sense to include it for the already existing ones.Testing
The change has been tested by
blockfrost-tests, and specifically:Summary by CodeRabbit
New Features
/accounts/{stake_address}/transactions).Bug Fixes
Tests