0xMiden / 0xMiden/faucet

bug: the issuance counter ignores pending MINT notes, so the faucet can promise more than max supply

Aberta
#290 4 comentários 0 reações 0 responsáveis Ver no GitHub
Linguagem predominante
Rust
Estrelas
10
Forks
12
Merge médio
1d 15h
PRs com merge (30d)
9

Descrição

### Summary

The faucet's issuance counter is only ever set from the faucet account's on-chain `token_supply`.
Since the faucet became a network account, that value does not move when the faucet submits a mint —
it moves later, when the network consumes the MINT note. Between those two moments the faucet
filters new requests against a supply figure that does not include what it has already promised, so
it can approve claims that take it past `max_supply`.

`filter_requests_by_supply` keeps a running total while it walks a batch, but that total is a local
variable and is never written back to `self.issuance`:

https://github.com/0xMiden/faucet/blob/next/crates/faucet/src/lib.rs#L657-L686

The only thing that ever writes the counter is `refresh_issuance`, which replaces it wholesale with
what the store currently reports:

https://github.com/0xMiden/faucet/blob/next/crates/faucet/src/lib.rs#L861-L867

and it is called immediately after the transaction is submitted:

https://github.com/0xMiden/faucet/blob/next/crates/faucet/src/lib.rs#L630-L631

At that point the MINT notes exist but have not been consumed, so the faucet account still reports
the old supply. `mint` says as much a few lines above:

> The faucet's transaction only creates the MINT notes; the P2ID notes are minted later by the
> network, so they never land in the client store.

Supply is enforced and updated inside the faucet's own mint procedure, which runs when the MINT note
is consumed rather than when it is created — in `miden-standards`,
`asm/standards/faucets/fungible.masm` asserts `amount <= max_supply - token_supply` and then writes
the new `token_supply` into the token config slot. So the check that would catch the overcommitment
happens strictly after the faucet has already answered the user.

### How this came about

This looks like a leftover from the network-account migration rather than a deliberate choice.

Before #262, `mint` built the P2ID notes directly and the **faucet itself** submitted the
transaction. Minting happened inside that transaction, so `token_supply` was already updated by the
time `refresh_issuance` read it back, and reading the account really was a single source of truth —
which is what #44 asked for.

After #262 the operator submits MINT notes and the network mints later, but `refresh_issuance` and
its call site came across unchanged, including the comment. The assumption they depend on is the
part that changed.

### Steps to reproduce

As a test against `next` in `crates/faucet/src/lib.rs`, which **passes today**. It builds the faucet
account with a `max_supply` of `150_000_000` and makes two claims of `100_000_000`, one per batch:

```rust
/// Submits a single claim as its own batch, the way separate requests arrive in production.
async fn claim(faucet: &mut Faucet, base_units: u64) -> Result {
let (tx_requests, rx_requests) = mpsc::channel(1);
let (sender, receiver) = oneshot::channel();
let request = MintRequest {
asset_amount: AssetAmount::new(base_units).unwrap(),
..mint_request()
};
tx_requests.send((request, sender)).await.unwrap();
drop(tx_requests);
faucet.run(rx_requests, 1).await.unwrap();
receiver.await.unwrap()
}

#[tokio::test]
async fn issuance_counter_ignores_pending_mint_notes() {
let store = /* SqliteStore in a temp dir */;
// Two claims of 100_000_000 each fit individually, but not together.
let mut faucet = build_faucet_on_chain(store, 0, 0, 150_000_000).await;

claim(&mut faucet, 100_000_000).await.expect("the first claim fits under the cap");

assert_eq!(
faucet.issuance.borrow().base_units(),
0,
"the promised amount is not recorded anywhere: the counter still reads the on-chain \
supply, which the network has not updated yet",
);

let second = claim(&mut faucet, 100_000_000).await;
assert!(
second.is_ok(),
"the faucet approved a second claim, taking issuance to 200_000_000 against a \
150_000_000 cap",
);
}
```

This needs `build_faucet_on_chain` to take the `max_supply` it builds the faucet account with, rather
than hard-coding `1_000_000_000_000`; existing callers pass the old value unchanged.

The companion control passes too — the **same two claims inside a single batch** are filtered
correctly, because `filter_requests_by_supply` carries its running total across one batch:

```rust
#[tokio::test]
async fn two_claims_in_one_batch_respect_max_supply() {
// ... both requests sent, then faucet.run(rx_requests, 2).await
assert!(results[0].is_ok(), "the first claim fits under the cap");
assert!(
matches!(results[1], Err(MintError::AvailableSupplyExceeded)),
"the second claim in the same batch is rejected, unlike the same claim one batch later",
);
}
```

So identical requests against an identical cap are handled correctly within a batch and incorrectly
across two. The batch boundary is the whole difference.

### Expected behaviour

The second claim is rejected with `AvailableSupplyExceeded`, as it is when both land in one batch.

### Actual behaviour

Both claims are answered successfully. The second is never minted, the user's proof of work is
spent, and the P2ID note id the faucet returned is cached and served by `get_note` for a note that
will never exist.

### Impact

Anyone claiming while the faucet is near its cap can be told they succeeded and receive nothing.
There is no error at the point of failure — the response has already been sent, and the MINT note
fails later inside a network transaction the user never sees.

To be clear about the limits: this is only harmful near `max_supply`. A faucet with a large cap and
low issuance over-approves harmlessly, because the claims still fit. What makes it more than
theoretical is that the window is not a narrow race — the counter is stale for every request between
submitting a batch and the network consuming it, which under load is the normal state rather than an
unlucky interleaving, and reaching it needs no unusual configuration.

What I have and have not verified: the over-approval above is **reproduced by running** the test
against the repo's own mock-chain harness. The final step — the network rejecting the second MINT
note on `amount <= max_supply - token_supply` — is **read** from `fungible.masm`, not observed, since
the mock chain does not consume MINT notes.

### Notes

There seem to be two directions, and they pull against each other, so I would rather ask than assume:

1. Track the promised amount locally — have `filter_requests_by_supply` commit its running total, and
reconcile with the account once the network catches up. Accurate immediately, but it reintroduces
the manual counter that #44 deliberately moved away from.
2. Keep the account as the single source of truth and account for the outstanding MINT notes
separately, so the filter subtracts what has been promised but not yet minted.

I am happy to implement either with regression tests. Which would you prefer?

Guia de contribuição

Abrir o guia de contribuição

Avaliação

Esta issue ainda não foi avaliada.

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.