faucet: block until the send lands and return the real txid - #55
Open
Giri-Aayush wants to merge 1 commit into
Open
faucet: block until the send lands and return the real txid#55Giri-Aayush wants to merge 1 commit into
Giri-Aayush wants to merge 1 commit into
Conversation
requestfaucetdonation queued the address and returned a fixed amount the instant it was called, before any transaction was built. Callers could not tell a paid request from a dropped one: the reply was success-shaped whether or not money moved, carried no txid to verify, and a failed build/broadcast was silently dropped from the queue with no retry. This makes it behave like wallet_staking_action, which already does the right thing a few hundred lines away: stage the request with a one-shot reply channel, and have the wallet loop answer only once the transaction actually reaches SENT, with the real txid, or on a definite failure, with an error. - FAUCET_STAGE mirrors STAKING_STAGE: (address, oneshot::Sender<Result<(u64, String), String>>). - The service handler blocks on the channel instead of calling a closure that returned immediately; a second concurrent request gets a busy error rather than silently queueing. - The wallet loop proposes the send from the miner wallet, drives it through build and broadcast, and replies with (amount, txid) on SENT or a real error on failure. A send that cannot select notes now reports that instead of reporting success. - FaucetResponse carries the txid; the zaino gRPC conversion reads .amount and is unaffected. Removes the now-unreachable FAUCET_REQUEST closure, FAUCET_Q queue, the idle-tick drainer, and the TEST_FAUCET fake-read, so there is one faucet path rather than two half-wired ones. The fixed amount and memo are unchanged.
8 tasks
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
1 participant
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.
tl;dr:
requestfaucetdonationreports success before it has actually sent anything, so a caller can't tell a real payout from a dropped one. This reworks it to behave likewallet_staking_action, which already does the right thing a few hundred lines away.How we found it
We were fronting this RPC from a faucet and noticed drips were getting recorded as "sent" while nothing showed up on chain. Scanned 500 blocks around a drip attempt and found zero faucet-shaped transactions. So we went into the source to see what the RPC actually does.
Turns out it decodes the address, pushes it onto a 16-slot queue, and returns
Ok(FAUCET_VALUE)immediately. No transaction has been built at that point. The reply is an "ok, queued" ack, not a receipt.FaucetResponseonly carriesamount, so there's no txid to verify against either. And the drainer that eventually builds the send advances its read cursor whether or not the send succeeded, so a failed build just quietly disappears.Net effect: the reply is success-shaped no matter what happens, which is exactly what we were seeing.
The fix (borrowed from staking)
The nice part is the same file already has a correct version of this pattern:
wallet_staking_action. It stages the request with a oneshot channel and blocks until the tx actually reachesSENT, then hands back the txid. We just mirrored it for the faucet.FAUCET_STAGEmirrorsSTAKING_STAGE:(address, oneshot::Sender<Result<(u64, String), String>>).(amount, txid)onSENTor a real error on failure. A send that can't select notes now says so.FaucetResponsegainstxid. The zaino gRPC conversion infetch.rsjust reads.amount, so it's unaffected.FAUCET_REQUESTclosure,FAUCET_Q, the idle-tick drainer, and theTEST_FAUCETfake-read, so there's one faucet path instead of two half-wired ones.Fixed amount and memo are unchanged.
Testing
Builds clean (
cargo build --release -p zebrad). Deployed to a live feature-net node: the RPC now returns a real error (no spendable orchard notes right now) exactly where it used to return a fake success. That's the honesty fix confirmed end to end. A full green-path drip is currently gated by a separate thing, the wallet rescanning from genesis on restart (#54), not by this change.