From 59da9a43f3d253408c8bf61823eeb1b42eefd1a7 Mon Sep 17 00:00:00 2001 From: maybay-dev Date: Mon, 31 Aug 2026 10:06:31 +0000 Subject: [PATCH] fix(factory): uncap `stream_addresses` to match paginated index page size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stream_addresses` (the batch ID→address resolver) was capped at `MAX_BATCH_SIZE` (10), the same limit used for write-heavy operations like `create_batch_streams` and `cancel_batch_streams`. Meanwhile, `streams_by_sender` / `streams_by_recipient` return up to 100 IDs per page (`MAX_PAGE_SIZE`). Resolving one full page of 100 stream IDs required 10 separate `stream_addresses` round-trips — contradicting the API's own docstring ("a page of IDs from either can be resolved to addresses in one call"). The root cause is that `stream_addresses` inherited the write-path cap (`MAX_BATCH_SIZE`) even though it only performs `persistent().get()` lookups per ID — no contract deployment, no token transfers, no governor cross-contract calls. The cost profile is purely read-bound. Introduce `MAX_RESOLVE_SIZE = 100` in `query.rs` as a dedicated read-path cap sized to match `MAX_PAGE_SIZE`, so a full page from either paginated index can be resolved in a single call. Add a new `ResolveTooLarge` error discriminant (30) so callers can distinguish a resolve-path rejection from a write-path `BatchTooLarge`. Alongside the core fix, this commit resolves several pre-existing build failures left by recent merges that prevented CI from passing: - `drip-stream/errors.rs`: Added missing `InvalidRecipient` (19), `BackdatedStream` (20), and `StreamUnderfunded` (21) variants that `drip-stream/src/lib.rs` references but `errors.rs` never defined. - `drip-factory/index.rs`: Fixed `streams_by_sender` return type from `Vec` to `StreamPage` and captured the `read_index` result as `ids` (both `streams_by_sender` and `streams_by_recipient` had the same bug where the return value was discarded). - `drip-factory/tests.rs`: Updated test assertions to access `page.ids.len()` / `page.ids.get()` instead of calling methods directly on `StreamPage`. Closes #418 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- contracts/factory/src/errors.rs | 2 ++ contracts/factory/src/index.rs | 6 +++--- contracts/factory/src/lib.rs | 21 ++++++++++++--------- contracts/factory/src/query.rs | 9 +++++++++ contracts/factory/src/tests.rs | 14 +++++++------- contracts/stream/src/errors.rs | 6 ++++++ 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/contracts/factory/src/errors.rs b/contracts/factory/src/errors.rs index 71a2c42c..f0fdf9c6 100644 --- a/contracts/factory/src/errors.rs +++ b/contracts/factory/src/errors.rs @@ -50,4 +50,6 @@ pub enum Error { /// is disabled there is no way to recover the funds before the stream /// starts. StartTimeTooFarInFuture = 29, + /// `stream_addresses` was called with more IDs than `MAX_RESOLVE_SIZE`. + ResolveTooLarge = 30, } diff --git a/contracts/factory/src/index.rs b/contracts/factory/src/index.rs index 7e712d74..bb5e4786 100644 --- a/contracts/factory/src/index.rs +++ b/contracts/factory/src/index.rs @@ -360,12 +360,12 @@ pub fn migrate_recipient_index(env: &Env, recipient: Address, max_pages: u32) -> .unwrap_or_else(|| env.storage().persistent().get(&count_key).unwrap_or(0)) } -pub fn streams_by_sender(env: &Env, sender: Address, offset: u32, limit: u32) -> Vec { +pub fn streams_by_sender(env: &Env, sender: Address, offset: u32, limit: u32) -> StreamPage { let count_key = DataKey::BySenderCount(sender.clone()); let legacy_key = DataKey::BySender(sender.clone()); let cursor_key = DataKey::BySenderMigrationCursor(sender.clone()); let legacy_count_key = DataKey::BySenderLegacyCount(sender.clone()); - read_index( + let ids = read_index( env, &count_key, &legacy_key, @@ -384,7 +384,7 @@ pub fn streams_by_recipient(env: &Env, recipient: Address, offset: u32, limit: u let legacy_key = DataKey::ByRecipient(recipient.clone()); let cursor_key = DataKey::ByRecipientMigrationCursor(recipient.clone()); let legacy_count_key = DataKey::ByRecipientLegacyCount(recipient.clone()); - read_index( + let ids = read_index( env, &count_key, &legacy_key, diff --git a/contracts/factory/src/lib.rs b/contracts/factory/src/lib.rs index bf5062e5..d8589fd5 100644 --- a/contracts/factory/src/lib.rs +++ b/contracts/factory/src/lib.rs @@ -24,7 +24,7 @@ use storage::DataKey; pub use storage::{BatchStreamRequest, FactoryStatus, FeeEstimate, StreamOperation, StreamPage}; /// Maximum number of streams accepted by a single `create_batch_streams` -/// (and `cancel_batch_streams`/`stream_addresses`) call. Each +/// (and `cancel_batch_streams`) call. Each /// `create_stream` in the batch performs a governor cross-contract call, /// two `token::transfer`s, a contract deploy + `initialize` invoke, and /// three persistent writes with TTL extensions (~2.5M CPU instructions). @@ -367,15 +367,18 @@ impl DripFactory { /// Batch-resolve stream IDs to their deployed contract addresses. /// - /// Pairs with `streams_by_sender`/`streams_by_recipient`: a page of IDs - /// from either can be resolved to addresses in one call instead of one - /// `stream_address` round-trip per ID. Unknown IDs resolve to `None` in - /// their slot, matching `stream_address`'s per-ID behavior, rather than - /// failing the whole batch. Capped at `MAX_BATCH_SIZE`, mirroring - /// `create_batch_streams`. + /// Pairs with `streams_by_sender`/`streams_by_recipient`: a full page of + /// IDs from either (up to [`query::MAX_PAGE_SIZE`]) can be resolved to + /// addresses in one call instead of one `stream_address` round-trip per + /// ID. Unknown IDs resolve to `None` in their slot, matching + /// `stream_address`'s per-ID behavior, rather than failing the whole + /// batch. Capped at [`query::MAX_RESOLVE_SIZE`] (100), sized for the + /// read path — this function only performs `persistent().get()` lookups + /// and never deploys contracts or transfers tokens, so the write-path + /// [`MAX_BATCH_SIZE`] does not apply. pub fn stream_addresses(env: Env, ids: Vec) -> Result>, Error> { - if ids.len() > MAX_BATCH_SIZE { - return Err(Error::BatchTooLarge); + if ids.len() > query::MAX_RESOLVE_SIZE { + return Err(Error::ResolveTooLarge); } let mut out = Vec::new(&env); for id in ids.iter() { diff --git a/contracts/factory/src/query.rs b/contracts/factory/src/query.rs index b35e7a10..bbfd07f1 100644 --- a/contracts/factory/src/query.rs +++ b/contracts/factory/src/query.rs @@ -5,6 +5,15 @@ use soroban_sdk::{Env, Vec}; /// sender's entire history in a single view call. pub const MAX_PAGE_SIZE: u32 = 100; +/// Hard cap for batch-resolving stream IDs to addresses. +/// +/// Sized to match [`MAX_PAGE_SIZE`] so that a full page returned by +/// `streams_by_sender` / `streams_by_recipient` (up to 100 IDs) can be +/// resolved in a single `stream_addresses` call. This is a read-only +/// operation (`persistent().get()` per ID) with no deployment or token +/// transfer, so the write-path [`MAX_BATCH_SIZE`] does not apply here. +pub const MAX_RESOLVE_SIZE: u32 = 100; + /// Returns a paginated slice of `v` starting at `offset` with at most `limit` /// elements. /// diff --git a/contracts/factory/src/tests.rs b/contracts/factory/src/tests.rs index 2960fd32..af95b7f0 100644 --- a/contracts/factory/src/tests.rs +++ b/contracts/factory/src/tests.rs @@ -282,9 +282,9 @@ fn legacy_sender_index_migration_is_incremental() { }); let page = s.client.streams_by_sender(&sender, &95, &10); - assert_eq!(page.len(), 10); - assert_eq!(page.get(0).unwrap(), 95); - assert_eq!(page.get(9).unwrap(), 104); + assert_eq!(page.ids.len(), 10); + assert_eq!(page.ids.get(0).unwrap(), 95); + assert_eq!(page.ids.get(9).unwrap(), 104); assert_eq!(s.client.migrate_sender_index(&sender, &10), 250); s.env.as_contract(&s.client.address, || { @@ -317,10 +317,10 @@ fn append_during_partial_sender_migration_preserves_order() { assert_eq!(s.client.stream_count_by_sender(&sender), 151); let tail = s.client.streams_by_sender(&sender, &145, &10); - assert_eq!(tail.len(), 6); - assert_eq!(tail.get(0).unwrap(), 145); - assert_eq!(tail.get(4).unwrap(), 149); - assert_eq!(tail.get(5).unwrap(), 999); + assert_eq!(tail.ids.len(), 6); + assert_eq!(tail.ids.get(0).unwrap(), 145); + assert_eq!(tail.ids.get(4).unwrap(), 149); + assert_eq!(tail.ids.get(5).unwrap(), 999); assert_eq!(s.client.migrate_sender_index(&sender, &10), 151); let tail_after = s.client.streams_by_sender(&sender, &145, &10); diff --git a/contracts/stream/src/errors.rs b/contracts/stream/src/errors.rs index c84f785e..3e1f8daa 100644 --- a/contracts/stream/src/errors.rs +++ b/contracts/stream/src/errors.rs @@ -22,4 +22,10 @@ pub enum Error { ReentrancyForbidden = 16, OperatorAlreadySet = 17, NotInitialized = 18, + /// The recipient is invalid (e.g. the all-zero Stellar account address, or identical to `sender`). + InvalidRecipient = 19, + /// The stream's `start_time` is in the past at initialization. + BackdatedStream = 20, + /// The stream has accrued tokens but is not funded enough to cover the requested withdrawal. + StreamUnderfunded = 21, }