Mobile Verifier Burner Txn Error check #906
Conversation
mobile_packet_verifier/src/burner.rs
Outdated
| .write(ValidDataTransferSession::from(session), &[]) | ||
| .await?; | ||
| let signature = txn.get_signature(); | ||
| for check_idx in 0..retry_attempts { |
There was a problem hiding this comment.
What is the expectation if the confirmation check fails each time ( for example due to solana congestion ) and exhausts the retry attempts ? Wont we be back in the same situation again whereby we potentially reburn a previously burnt balance
There was a problem hiding this comment.
i chatted with @bbalser on this and we both agree this is a gap that cannot be solved readily without some additional structural changes. This PR is still an improvement on what was there previously.
There was a problem hiding this comment.
I think the next pass at this will most likely include some block height checking. That way we can be tell the difference between solana errors and the amount of time passing without our txn on being on chain.
If a submitted transaction does not come back as a defacto success, we spawn a process that checks a txn made it to the chain by signature. By default, we will retry 5 times a minute apart, before considering the txn an actual failure to be tried again later. If during the checking we find the txn on chain, we do the same logic as if it had passed the first time.
michaeldjeffrey
force-pushed
the
mj/burner-error-check
branch
from
6edd596 to
2db933d
Compare
mobile packet verifier is not handling many burn txns, we can block on confirming before attempting to process the next txn. Txn tracking by signature and block-height will come in a later update.
michaeldjeffrey
force-pushed
the
mj/burner-error-check
branch
from
8b059ec to
af13305
Compare
| // We have failed to burn data credits: | ||
| metrics::counter!( | ||
| "burned", | ||
| "payer" => payer.to_string(), |
There was a problem hiding this comment.
do we want to include the payer? this is either just the one service provider or an (potentially) unbounded list of payers that balloons the metrics cost by increasing the breadth of the monitored dataset, right?
There was a problem hiding this comment.
we already include payer in this metric today. There could be more wallets in the future that pay for this, but we have some alerts that require payer be in this metric
Summary
When submitting a transaction with the Solana client, there is a chance an error is returned but the transaction still made it to the chain.
In this first pass, we're taking a verify approach. When any error is returned, we spawn a task that attempts to confirm the transaction a number of times before really considering it a failure.
A later PR will come in to implement proper transaction tracking.
Changes
txn_confirmation_retry_attemptstxn_confirmation_check_intervalSolanaNetworknow requires implementingClone