Persisting newly-broadcast transactions locally

[kyranjamie]

Opening this discussion to house everyone's thoughts regarding the wallet functionality where, newly-broadcast, or pending, transactions are persisted on the client.

We've discussed this verbally in the UserX team. @diwakergupta floated the idea on Discord as well.

This functionality improves:

• User Xperience. Transactions appear in the UI immediately. Currently we refetch the mempool after broadcast, which takes, say, ~3 seconds
• Network resilience. Slow network conditions, or API/infrastructures issues such hirosystems/devops#797, have less of a visible impact on the wallet's behaviour

On the face of it, it seems obvious. And in some capacity, I think we'll need this behaviour. Though, I have concerns there are cases where the wallet, and the actual state of the chain, might diverge. Resulting in us displaying confusing or wrong information.

Questions

• Can we be absolutely certain that a transaction has broadcast successfully and will be picked up by the mempool/included in a block when POST /v2/transactions returns a 200 response?
• With congested network conditions, in what time can we expect a broadcast transaction to appear in the mempool?
• After what duration of a transaction being broadcast and not having appeared in the mempool or anchor block do we consider it stale?
• Are there other circumstances where a transaction could be broadcast and not picked up by a node?

I built this functionality in the desktop wallet pre the existence of the /mempool endpoint. But ended up removing it. It was a complex implementation, and not necessary if the mempool displays the current state "pending txs". I'd like to understand better to what extent we can rely on the mempool to be accurate. Prior to this current issue, it's worked pretty well. I haven't seen wallet issues related to our reliance on the mempool.

Regarding slow UX: To avoid a post-broadcast transactionless UI (where's the tx I just sent?!), the desktop wallet waits (with a timeout) for it first to appear in the mempool while the transaction is still "Sending...". This slows down the experience a little, but usually it takes no more than a couple of seconds. It'd be great to do it 2 seconds faster, but how much time/effort/implementation complexity is that worth?

While caching pending txs might help us in some instances of network instability, such as a tx never entering mempool, couldn't the same instability prevent us from ever knowing what to do with said transaction, and make the experience worse?

Consider that our implementation must:

• Deduplicate transactions by txid (so UI doesn't ever display mempool & local cache)
• Monitor mempool & anchor block transactions to remove matching txids from cache
• Include a mechanism to remove from the cache after a certain amount of time since broadcast and it not being detected

(We also have the ambition to simplify our state mgmt set up, before starting work that might further complicate it)

This is simply to say given that its a non-trivial feature to build, we should consider the value it adds, and its implementation, before jumping into development.

cc/ people who might have input/be able to answer questions @aulneau @lgalabru @zone117x @wileyj @jcnelson @kantai

[markmhendrickson]

Might it be cleanest for the UX and code alike to introduce a new “Submitted” state for all transactions that have received a 200 from the API upon broadcast but haven’t yet been seen in the mempool as indicated by the API, upon which time they’d switch to “Pending”?

We could show submitted transactions immediately and indefinitely since this remains an accurate representation (they are indeed submitted), without getting into any limbo that could result from (over-) eagerly representing them as pending.

If a submitted transaction hasn’t been seen in the mempool for awhile, we could optionally flag as “Looks like this transaction is having problems” and the like, as progressive enhancements.

[kyranjamie]

I like this idea. A new "submitted" state vs. "mempool simulation" is an important distinction, and definitely keeps the implementation simpler.

Seeing the UI update as the tx goes from submitted → in mempool would itself make it seem faster, than just seeing it straight in pending. omg something's happening 😱

[fbwoolf]

Using optimistic updates with react-query might be helpful to implement something like this. There are built-in handlers for onError and onSettled to rollback or invalidate the query depending on if the tx is successful.

https://react-query.tanstack.com/guides/optimistic-updates

[fbwoolf]

However, I worry a bit that we might cover up issues that are actually helpful to expose?

[aulneau]

However, I worry a bit that we might cover up issues that are actually helpful to expose?

Can you say more about this?

[fbwoolf]

My point is just that when users report issues in the wallet/explorer it can often help reveal problems that need to be addressed with the api.

[aulneau]

This is a great write up @kyranjamie!

I generally agree, and it's important to think through how this might work in the wallet. I agree with @markmhx on how there should be a new state of submitted, where it's not trying to replicate mempool state.

In my mind this is really a shift of where or how we're constructing data. Currently we basically rely exactly on the endpoint to construct the shape of our state, eg we fetch a paginated list of transaction activity and then display it. I think if we shift that and instead have the basic building block of our "activity" be an array of txids, then we no longer need to rely on the shape of data coming from the transaction array api endpoint. To illustrate:

currently

• user is signed in
• we fetch both the pending and confirmed transactions for the user
• we display a list of items from this data:
• (MempoolTransaction | Transaction)[].map(tx => <TxComponent tx={tx} />)

my suggestion

• user is signed in
• we fetch both the pending and confirmed transactions for the user
• we sync any local transactions to a new array of this type: {txid: string; txRaw: string, timestamp: number} (not sure about needing a timestamp)
• we create a "submitted" version of the transaction, where we parse the raw tx to a StacksTransaction (with the addition of the txid)
• we map over the pending/confirmed transactions like this [...pendingTxs, ...confirmedTxs].map(tx => tx.tx_id) (or use a record)
• we map over the local transactions like this localTxs.map(tx => tx.txid) (or use a record)
• we combine them, so it's one array (or record) [...new Set([...localTxIds, ...pendingTxids, ...confirmedTxids])]
• we shift the responsibility of fetching data to the TxComponent, by passing it a txid instead of a transaction
• the default behavior of the tx component would be to fetch data for the tx by txid
• while it's suspended, we can use a fallback component that uses the local tx data

With this, we'd end up with kind of user experience:

• user submits a transaction
• transaction gets persisted locally
• the txid is added to the list of txids that already exist immediately
• the TxComponent mounts and suspends immediately, because it's trying to fetch data from the tx endpoint
• while it's suspended (waiting for the tx to show up in the mempool), the fallback will use the data from the parsed raw transaction (as a StacksTransaction, our "submitted" state)
• when the txid shows up in the mempool, the fallback is removed and we see a pending transaction
• when the tx confirms (or fails) it updates automatically

We'd also be able to use these raw transactions to subtract balance and calculate nonces better.

DIdn't mean to get too into the weeds! Let me know if I can explain anything differently or expand on anything.

[fbwoolf]

Got it, thanks for clarifying, that makes sense. 👍

[kyranjamie]

Good ideas, thanks Thomas. Happy to talk through implementation.

The uncertainty I have lies with whether or not to pass off all the fetching to <TxComponent/>, or preparing the data first. This suggestion makes a nice API, but there are some downsides. For example, for every tx component instance, it has to check all 3 places where a tx exists, right? I don't think it's super important to use suspense here, but what if the local tx data isn't cached?

Where might the logic to recurrently check for, and remove, stale transactions live?

[aulneau]

it has to check all 3 places where a tx exists, right?

There would only be two ways for a txid to exist:

• it comes from the accounts transactions endpoints -- so either pending or confirmed, which as the wallet is built currently, this can be consumed as one atom
• or, it comes from locally set data -- the result of a successful broadcast.

Where might the logic to recurrently check for, and remove, stale transactions live?

With what I suggested, you'll never need to do any work to check or remove any transaction ids, because if the txid is found both in local data, and external data, duplications will be removed by this line in my above post: [...new Set([...localTxIds, ...pendingTxids, ...confirmedTxids])] (and there are potentially smarter ways of doing this).

Since we are already using react-query, either as an atom or a useQuery hook, all the fetching and re-fetching will happen there. Here is an example component:

const ActivityListComponent = () => {

    // these would come from atoms or queries
    const localTxIds: string[] = [];
    const pendingTxIds: string[] = [];
    const confirmedTxIds: string[] = [];

    const allTransactionTxIds: string[] = [...new Set([
        ...localTxIds,
        ...pendingTxIds,
        ...confirmedTxIds
    ])];

    return <>{allTransactionTxIds.map(txid => <TxComponent txid={txid} key={txid}/>)}</>;
};


const TxComponent = ({txid}) => {
    // the client state as it's currently implemented
    const [client] = useAtom(apiClientState)

    // assume this is record of string, StacksTransaction
    const localTransactions: Record<string, StacksTransaction> = useAtom(localTransactionsState)

    // a standard react-query hook (could use an atom too)
    const {data, state} = useQuery(['single-transaction', txid], () => {
        return client.transactionsApi.getTransactionById({
            txId: txid
        })
    })

    // would likely need an error state here
    // if(state === 'hasError') etc

    // if we have data, return the existing tx component that accepts data
    if(data) return <ActualTransaction data={data} />
    
    // if we have a local tx, return that
    if(localTransactions['txid']) return <SubmittedTransaction data={localTransactions['txid']} />
    
    // this state should technically be impossible, but never hurts to have a fallback
    return <SomeLoadingTxComponent />
}

[fbwoolf]

Would there be any benefit to handling confirmed transactions separately? Those will always be confirmed, correct? The states we are most concerned with are submitted (waiting to get picked up) and pending (picked up in mempool). Is it faster just to get all the confirmed transactions as a list and then handle the pending transactions as single id queries?

[beguene]

+1 I think keeping track of the tx submitted locally is a good idea. It will also help us calculate nonce better and prevent some nonce issues. For now this merge of all 3 tx states is simple and good enough. But in the future we might have a lot of confirmed tx for heavy users, so we will have to optimize.