feat(funding-service): collect deposits sent to the funding account + worker refactor by SantiagoPittella · Pull Request #2615 · 0xMiden/node

SantiagoPittella · 2026-09-11T21:00:37Z

Summary

Adds funding for the funder account via a public pay-to-ID note holding the native asset, and the service finds and consumes it on its own.

Only public, pay-to-ID, targeting the funding account, holding nothing but the native asset are collected.

Deposits are consumed in their own transaction, so a note that turns out to be unconsumable cannot fail a request a client is waiting on.

--top-up-interval controls how often it runs, defaulting to one minute.

Changelog

[[entry]]
scope       = "funding-service"
impact      = "added"
description = "The service collects public pay-to-ID deposits sent to its funding account."

kkovaacs · 2026-09-14T14:59:41Z

+                .sync_nullifiers(SyncNullifiersRequest {
+                    block_range: Some(BlockRange {
+                        block_from: BlockNumber::GENESIS.as_u32(),
+                        block_to: tip.as_u32(),


This is always querying the full block range up to the tip for spent nullifiers. Can't we possibly use a shorter range? We already seem to be tracking the block number we've caught up with when finding deposits. Since we're the only consumer of those deposit notes can't we assume that those can't be spent before that block number?

Yes, we could use a block from here

We could also decide not to scan nullifiers and base everything off the note scan only. A bit more risky/error prone maybe, but should be manageable.

igamigo

Mostly some naming nits (feel free to disregard), and a couple of general comments. Because this submits transactions independently of the normal service, there are some race conditions involved here, so I wonder if transaction environments should be shared: consumption and funding could share the same transaction and so there would be no races. I think this was mentioned somewhere so maybe you decided against it for a reason, so feel free to disregard.

There are also some mostly harmless races because the chain tip/accounts/blockchain are queried at different times (spent_nullifiers and transaction_is_pending can be querying different heights), but this is very minor. I wonder if the general flow of a single service tick could change to be like so: you fetch the latest block once, sync transactions (and check whether a pending one was submitted/discarded), sync notes (optionally nullifiers as well) and store all the new state up to the chain tip. Then, the funder executor could take any number of these notes whenever it needs to fund a new account, and submit a transaction that both funds the account and consumes a small amount of notes (the ones that have the most tokens in them). I think this could make the flow easier to follow, but maybe this is too much of a refactor and the status quo is good enough so no need to do anything now.

igamigo · 2026-09-15T20:18:48Z

@@ -1,6 +1,6 @@
 //! Node access. The RPC handling is copied from the network monitor.


nit: Noticed this mentions the setup is copied, feels unnecessary for the module docs

igamigo · 2026-09-15T20:19:25Z

        fetch_public_account(&mut self.rpc_client.clone(), account_id, block_num).await
    }

+    /// The IDs of the committed notes which carry `tag`, from `from_block` up to the chain tip.


nit:

Suggested change

/// The IDs of the committed notes which carry `tag`, from `from_block` up to the chain tip.

/// Fetches the IDs of the committed notes which carry `tag`, from `from_block` up to the chain tip, alongside the current chain tip.

igamigo · 2026-09-15T20:22:57Z

+    }
+
+    /// The notes among `note_ids` whose details the node stores.
+    pub async fn public_notes(&self, note_ids: &[NoteId]) -> Result<Vec<Note>> {


nit: I'd name this get_public_notes_by_id

igamigo · 2026-09-15T20:29:42Z

+                .sync_nullifiers(SyncNullifiersRequest {
+                    block_range: Some(BlockRange {
+                        block_from: BlockNumber::GENESIS.as_u32(),
+                        block_to: tip.as_u32(),


We could also decide not to scan nullifiers and base everything off the note scan only. A bit more risky/error prone maybe, but should be manageable.

igamigo · 2026-09-15T20:32:24Z

+        if deposits.is_empty() {
+            return Ok(());
+        }


Do we want to log anything here?

igamigo · 2026-09-15T21:01:15Z

+    let input_notes =
+        InputNotes::new(deposits.into_iter().map(InputNote::unauthenticated).collect())
+            .context("failed to build the deposit input notes")?;


I think we could stall the service indefinitely here if we don't cap the number of notes that we pass to the execution environment. I would probably order them by amount and take maybe the top 20 or so, probably no need to do a lot more than that. Not sure what the limit is but we probably want to be on the conservative side considering it increases proving time, etc.

igamigo · 2026-09-15T21:03:40Z

+    }
+
+    /// The nullifiers among `nullifiers` which are already spent.
+    pub async fn spent_nullifiers(&self, nullifiers: &[Nullifier]) -> Result<HashSet<Nullifier>> {


nit: To be in line with the sync_note_ids call I'd name this sync_nullifiers()

igamigo · 2026-09-15T21:06:07Z

+        return false;
+    }
+
+    note.assets().num_assets() == 1 && native_amount(note, fee_faucet_id) > 0


One thing to keep in mind here is that technically people could potentially grief and send 1 absolute native token per note. In these cases a single note would not cover the fees for a single transaction, so we would need to take care to submit if the net is positive. Maybe this will never be a problem, but also it should not be hard to address

igamigo · 2026-09-15T21:11:12Z

+            return Ok(false);
+        }
+
+        if self.node.committed_tip().await? >= expiration_block {


nit: sync_nullifiers already implicitly carries the chain tip/block_to so maybe that can get reused

igamigo · 2026-09-15T21:12:27Z

+
+    /// Reports whether the submitted transaction is still pending.
+    async fn transaction_is_pending(&mut self) -> Result<bool> {
+        let Some((nullifiers, expiration_block)) = self.pending.clone() else {


nit: Probably can use self.pending.is_some() to avoid cloning the list of nullifiers here.

Mirko-von-Leipzig · 2026-09-16T05:59:32Z

Mostly some naming nits (feel free to disregard), and a couple of general comments. Because this submits transactions independently of the normal service, there are some race conditions involved here, so I wonder if transaction environments should be shared: consumption and funding could share the same transaction and so there would be no races. I think this was mentioned somewhere so maybe you decided against it for a reason, so feel free to disregard.

There are also some mostly harmless races because the chain tip/accounts/blockchain are queried at different times (spent_nullifiers and transaction_is_pending can be querying different heights), but this is very minor. I wonder if the general flow of a single service tick could change to be like so: you fetch the latest block once, sync transactions (and check whether a pending one was submitted/discarded), sync notes (optionally nullifiers as well) and store all the new state up to the chain tip. Then, the funder executor could take any number of these notes whenever it needs to fund a new account, and submit a transaction that both funds the account and consumes a small amount of notes (the ones that have the most tokens in them). I think this could make the flow easier to follow, but maybe this is too much of a refactor and the status quo is good enough so no need to do anything now.

I suspect this is perhaps the direction to go. The main reason is that this service may see more traffic than we initially expected. This is due to now also using this to supply initial funds to user accounts upon registration.

What I was thinking is perhaps a dedicated task which ticks similarly as described by @igamigo above. Though I would perhaps only ever have a single transaction inflight? Input to this task could be via a channel for rpc requests. Collecting the top-up notes could be part of this sync flow.

I think we should merge the first three PRs in this stack so long, just to get them merged. Our initial design here was insufficient (my bad); so this may take some further work imo.

I wonder if the client flow should therefore look a bit different as well. We initially wanted the note information to be sent back, but that will be a problem if we include retries etc. If we timeout as part of account registration then this is a bit awkward - perhaps its good enough for this service to acknowledge the request and guarantee that it will, at some point, create the note?

A further hardening step would then be adding a small database to track accounts-to-fund. But that can be a separate PR.

igamigo · 2026-09-16T12:50:29Z

I wonder if the client flow should therefore look a bit different as well. We initially wanted the note information to be sent back, but that will be a problem if we include retries etc. If we timeout as part of account registration then this is a bit awkward - perhaps its good enough for this service to acknowledge the request and guarantee that it will, at some point, create the note?

I was always thinking the response would be optimistic: we would send the note details or note ID back so that users can know that will exist in the next few moments. Users can still try to consume the note as unauthenticated or poll whether the ID exists on chain to make sure. Also, if notes are public then wallets or clients will get the note naturally when syncing which I think was Bobbin's intention (here maybe we should make sure the UI communicates this somehow but it's an app-level problem).

Mirko-von-Leipzig · 2026-09-16T12:53:11Z

I wonder if the client flow should therefore look a bit different as well. We initially wanted the note information to be sent back, but that will be a problem if we include retries etc. If we timeout as part of account registration then this is a bit awkward - perhaps its good enough for this service to acknowledge the request and guarantee that it will, at some point, create the note?

I was always thinking the response would be optimistic: we would send the note details or note ID back so that users can know that will exist in the next few moments. Users can still try to consume the note as unauthenticated or poll whether the ID exists on chain to make sure. Also, if notes are public then wallets or clients will get the note naturally when syncing which I think was Bobbin's intention (here maybe we should make sure the UI communicates this somehow but it's an app-level problem).

Sure so long as the request handler creates the note, and then returns it quickly without waiting on the note to commit?

SantiagoPittella · 2026-09-16T17:50:31Z

So, before start changing things and after reading the comments. I think that the service should be something like this:

For the funding requests:

sequenceDiagram
    actor User
    participant Server
    participant Worker
    participant RPC

    rect rgb(235, 244, 255)
        Note over User,RPC: Request
        User->>Server: POST(ID, amount)
        Server->>Server: Create public note
        Server->>User: serialized note
        Server--)Worker: QUEUE(note)
    end

    rect rgb(235, 250, 238)
        Note over User,RPC: Processing
        Worker->>Worker: COLLECT NOTES
        Note over Worker: top-up notes first,<br/>then queued notes up to the tx limit
        Worker->>RPC: SubmitTransaction(input notes = top-ups, output notes)
        RPC->>Worker: BlockNumber
    end

    rect rgb(255, 247, 232)
        Note over User,RPC: Consumption
        User->>RPC: SubmitTransaction(input notes)
        RPC->>User: BlockNumber
    end

For the top up requests:

sequenceDiagram
    actor Depositor
    participant Server
    participant Worker
    participant RPC

    Depositor->>RPC: SubmitTransaction(output note = top-up to the funder)

    loop every interval
        Server->>RPC: SyncNotes(tag, from cursor)
        RPC->>Server: note ids, last checked block
        Server->>RPC: GetNotes(note ids)
        RPC->>Server: notes
        Server->>Server: keep public P2ID notes to the funder,<br/>native asset only
        Server->>RPC: CheckNullifiers(nullifiers)
        RPC->>Server: spent nullifiers
        Server->>Server: drop spent notes, advance cursor
        Server--)Worker: QUEUE(top-up note)
    end

    Worker->>Worker: COLLECT NOTES
    Note over Worker: top-up notes are drained first
    Worker->>RPC: SubmitTransaction(input notes = top-ups, output notes)
    RPC->>Worker: BlockNumber

Is this approach correct? @Mirko-von-Leipzig @igamigo

Mirko-von-Leipzig · 2026-09-16T18:07:06Z

@SantiagoPittella that looks correct; the other side to consider is whether a note ID is sufficient or whether the full note is required for ease of use. I'm not actually certain myself; the sequencer/account-registration only needs an ack - but the infrastructure might be much easier with a full note.

igamigo · 2026-09-16T18:29:05Z

Sure so long as the request handler creates the note, and then returns it quickly without waiting on the note to commit?

Yeah, exactly. Basically I think we can execute the transaction and then return the note to the user before going on to prove and submit the transaction.

Is this approach correct?

That's about what I was suggesting. Basically use the task to sync available notes, and use them on the transactions that we already submit. I would change in the diagram that maybe we could return the note details after having executed the transaction but before proving it (or maybe after proving it but before submitting? I think the degree of confidence that the tx will get committed at that point is significant).

One thing I'm not sure about: is the plan to collect user requests and resolve multiple of them in a single transaction or is it currently one transaction per request? Mostly considering the increase in usage from the account registration process. Handling multiple request in one transaction sounds like an optimization and one transaction per request is easier on the HTTP side, but then you need to track how the account changes as transactions get committed (unless you want to wait until the transaction gets fully committed which implies waiting for at least one block). We could be stacking 3 transactions and then find out that the first one did not commit so you'd need to redo all of them.

the other side to consider is whether a note ID is sufficient or whether the full note is required for ease of use. I'm not actually certain myself; the sequencer/account-registration only needs an ack - but the infrastructure might be much easier with a full note.

I would go with responding with full note details as this should not be prohibitive in any way (AFAICT), and the account whitelisting flow can discard it in any case. For infra/integration tests notes are more useful: you can consume them as unauthenticated and you dont have to poll the node to get the note details which can waste a couple seconds.

Mirko-von-Leipzig · 2026-09-16T18:38:51Z

One thing I'm not sure about: is the plan to collect user requests and resolve multiple of them in a single transaction or is it currently one transaction per request? Mostly considering the increase in usage from the account registration process. Handling multiple request in one transaction sounds like an optimization and one transaction per request is easier on the HTTP side, but then you need to track how the account changes as transactions get committed (unless you want to wait until the transaction gets fully committed which implies waiting for at least one block). We could be stacking 3 transactions and then find out that the first one did not commit so you'd need to redo all of them.

I was sort of thinking we do:

Request handler creates only the note (I assume this is possible without having locked account access?).
This lets the request handler immediately return the note ID/details without waiting on the rest of the transaction process.
Handler sends the note to the main task via channel.
Main task can then independently decide on how to process however it wants.
- It could process one note at a time, or
- process all queued notes in one batch
  - either creating a tx per interval or
  - just whatever is in the queue at the time

The downside of course is that the guarantees are a lot lower than if we wait until the tx is executed or proven..

SantiagoPittella · 2026-09-17T14:35:54Z

I was sort of thinking we do:

Request handler creates only the note (I assume this is possible without having locked account access?).

This lets the request handler immediately return the note ID/details without waiting on the rest of the transaction process.

Handler sends the note to the main task via channel.

Main task can then independently decide on how to process however it wants.

It could process one note at a time, or

process all queued notes in one batch

either creating a tx per interval or

just whatever is in the queue at the time

The downside of course is that the guarantees are a lot lower than if we wait until the tx is executed or proven..

Yeah, this is what I thought too. The sequence diagram that I posted is essentially that, the server/handler immediately returns the account to the user and then just queues it to the worker which polls N notes and creates the tx

SantiagoPittella · 2026-09-17T18:40:31Z

As per the comments, I did a commit here with the refactor. Let me know if you guys prefer a new PR with this commit b3f5aef

It includes:

One worker to process notes and it is the only allowed to create transactions (theTopUpCollector is gone). Deposits are consumed as the input notes of the same transaction that creates the funding notes.
The handler answers immediately: the handler builds and returns it, and queues it. The response is now { "note": "..." }, meaning that the inclusion proof and transaction ID are gone, because neither exists when the service answers.
The caller already holds the note, so an expired transaction requeues it with the same serial number, rather than failing the request.
Limits: At most 16 deposits per transaction

Mirko-von-Leipzig

Not a super deep review; but I think this looks good.

Mirko-von-Leipzig · 2026-09-21T11:29:12Z

I do think we have some race conditions, or failures that can sneak through.

For example, I think if we fail to submit a transaction (e.g. timeout) but the node accepts it, then we retry the same deposit notes indefinitely?

We may need a database to do this rigorously I think.

igamigo

Looks good. Leaving some nits, and just a few comments we may want to address before merging.

igamigo · 2026-09-21T20:12:33Z

+    /// Where a handler puts the note it built, for the worker to create.
+    pub(crate) requests: mpsc::Sender<Note>,


nit: Technically we need very few details from each note. Specifically, the serial number and note storage should suffice AFAIK. Everything else should remain static and so we can make the communication channel much leaner. Not sure this optimization is worth it at all though (probably not)

Hmm, I don't think there is much benefits in not using the full note here, and avoid re-creating it after. This way we just create it once and queeu it/return to the client

Yeah, as mentioned above I think in terms of size it could be significant (NoteScript is large and we'd be keeping multiple copies of it) but functionally probably no difference 🤷

igamigo · 2026-09-21T20:24:40Z

+        tag: NoteTag,
+        from_block: BlockNumber,
+    ) -> Result<SyncedNotes> {
+        let tip = self.committed_tip().await?;


I think we want to fetch this once, outside of all the sync calls, and use that as a parameter to avoid races where the tip advances between calls

I created d2c0e53 which addresses this and some. ofthe next comments

igamigo · 2026-09-21T20:30:57Z

+    let mut tx_args = if outputs.is_empty() {
+        let script = ExpirationTransactionScript::new(expiration_delta);
+        let script_args = script.tx_script_args();
+
+        TransactionArgs::default().with_tx_script_and_args(script.into(), script_args)
+    } else {


I think I would disallow generating transactions if no outputs are expected. I think it simplifies assumptions and you avoid blocking the account with a transaction that only funds the account when an actual user comes and requests a note (cc @Mirko-von-Leipzig). Not a strong opinion though.

Something to have in mind is that. wecheck the balance of the account when a new funding request arrives, and if we don;t have enough balance we reject the request. Thus this can have us to freeze the account because wont we consuming the top up because we are rejecting. the funding. Though we can change that check

igamigo · 2026-09-21T20:34:53Z

+        let from_block = self.next_block;
+        let tag = NoteTag::with_account_target(self.funder);
+        let synced = node.sync_note_ids(tag, from_block).await?;
+        self.next_block = synced.last_checked_block + 1;


If the RPC calls fail then we will have advanced the cursor without having received the data here

#2615 (comment)

igamigo · 2026-09-21T20:36:03Z

+    pub async fn sync_note_ids(
+        &self,
+        tag: NoteTag,
+        from_block: BlockNumber,
+    ) -> Result<SyncedNotes> {


This is returning just one page per scan, no?

#2615 (comment)

SantiagoPittella mentioned this pull request Sep 11, 2026

feat(funding-service): collect deposits sent to the funding account #2604

Closed

SantiagoPittella changed the title ~~Fs/4 account top up v2~~ feat(funding-service): collect deposits sent to the funding account Sep 11, 2026

SantiagoPittella added this pull request to stack #2616 September 11, 2026 21:02

SantiagoPittella requested review from Mirko-von-Leipzig and igamigo September 11, 2026 21:02

Mirko-von-Leipzig reviewed Sep 14, 2026

View reviewed changes

Comment thread bin/funding-service/src/top_up.rs Outdated

Comment thread bin/funding-service/src/top_up.rs Outdated

Comment thread bin/funding-service/src/top_up.rs Outdated

Comment thread bin/funding-service/src/top_up.rs Outdated

kkovaacs reviewed Sep 14, 2026

View reviewed changes

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch 3 times, most recently from 9b4af3e to dfc5474 Compare September 15, 2026 13:11

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch 2 times, most recently from 760e2be to 2117978 Compare September 15, 2026 19:06

SantiagoPittella requested review from Mirko-von-Leipzig and kkovaacs September 15, 2026 19:44

igamigo reviewed Sep 15, 2026

View reviewed changes

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch from 2117978 to f99f94a Compare September 16, 2026 13:52

Base automatically changed from fs/3-network-and-monitor to next September 16, 2026 15:03

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch from f99f94a to aab7e82 Compare September 16, 2026 15:03

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch from aadf30f to b3f5aef Compare September 17, 2026 18:36

SantiagoPittella changed the title ~~feat(funding-service): collect deposits sent to the funding account~~ feat(funding-service): collect deposits sent to the funding account + worker refactor Sep 17, 2026

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch from b3f5aef to 41d570a Compare September 18, 2026 13:37

Mirko-von-Leipzig approved these changes Sep 21, 2026

View reviewed changes

SantiagoPittella added 9 commits September 21, 2026 08:54

feat(funding-service): collect deposits sent to the funding account

7cfd131

review: rename to p2id_collection

2c9dd35

review: impose limit on nullifiers

c01a621

review: rename to TopUpCollector

5a6db3e

review: public fields

fd493c1

review: use tokio::interval

c7abbf2

chore: update with latest protocol changes

5bcf5ae

review: wait for spent note

314bee1

refactor: create worker task to process notes

ea86703

SantiagoPittella force-pushed the fs/4-account-top-up-v2 branch from 41d570a to ea86703 Compare September 21, 2026 11:54

SantiagoPittella requested a review from igamigo September 21, 2026 11:56

Merge branch 'next' into fs/4-account-top-up-v2

a3a77ae

igamigo reviewed Sep 21, 2026

View reviewed changes

Mirko-von-Leipzig and others added 5 commits September 22, 2026 17:46

Merge next into fs/4-account-top-up-v2

34b1fd9

review: rename to deposit

8edddf3

review: validate funded address

aad302c

review: refactor page fetch

d2c0e53

review: use AssetId instead of AccoutnId

c7de714

Mirko-von-Leipzig merged commit 9eaa599 into next Sep 23, 2026
30 checks passed

Mirko-von-Leipzig deleted the fs/4-account-top-up-v2 branch September 23, 2026 06:37

SantiagoPittella mentioned this pull request Sep 23, 2026

Create a funding service #2561

Closed

		@@ -1,6 +1,6 @@
		//! Node access. The RPC handling is copied from the network monitor.

	/// The IDs of the committed notes which carry `tag`, from `from_block` up to the chain tip.
	/// Fetches the IDs of the committed notes which carry `tag`, from `from_block` up to the chain tip, alongside the current chain tip.

		/// Where a handler puts the note it built, for the worker to create.
		pub(crate) requests: mpsc::Sender<Note>,

Conversation

SantiagoPittella commented Sep 11, 2026

Summary

Changelog

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igamigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mirko-von-Leipzig commented Sep 16, 2026

Uh oh!

igamigo commented Sep 16, 2026

Uh oh!

Mirko-von-Leipzig commented Sep 16, 2026

Uh oh!

SantiagoPittella commented Sep 16, 2026

Uh oh!

Mirko-von-Leipzig commented Sep 16, 2026

Uh oh!

igamigo commented Sep 16, 2026

Uh oh!

Mirko-von-Leipzig commented Sep 16, 2026

Uh oh!

SantiagoPittella commented Sep 17, 2026

Uh oh!

SantiagoPittella commented Sep 17, 2026

Uh oh!

Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

Uh oh!

Mirko-von-Leipzig commented Sep 21, 2026

Uh oh!

igamigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

igamigo Sep 21, 2026 •

edited

Loading