tx: Remove wait_for_in_block helper method

[lexnv]

The wait_for_in_block waits until the transaction stream produces a TxStatus::InBestBlock event.

Transitioning to the latest RPC-V2, mainly using the transaction and chainHead, there is no strong guarantee that a new block reported by the transaction method will ever be reported by the chainHead.
This edge case could happen when the transaction API encounters a best block that is immediately pruned, possibly leading to the chainHead not gaining visibility for the said pruned block.

To offer stronger guarantees, the subxt submit_transaction ensures that a block reported by TxStatus::InFinalizedBlock is accompanied by a chainHead::Finalized event.

Users who may still want the same behavior for testing purposes could then:

let mut progress: TxProgress = ..;

while let Some(status) = self.next().await {
    match status? {
        TxStatus::InBestBlock(s) | TxStatus::InFinalizedBlock(s) => return Ok(s),

        // Error scenarios; return the error.
        TxStatus::Error { message } | TxStatus::Invalid { message } | TxStatus::Dropped { message } => {
            // treat error
        }
        // Ignore anything else and wait for next status event:
        _ => continue,
    }
}

Motivated by: #1236

cc @ascjones this might be of interest for contracts

[niklasad1]

I still find this useful for instance on some scenarios such as the staking miner then one could select "best blocks" instead of "finalized block" but yeah it can be a footgun

[jsdw]

I think this is a good way to go; we don't remove the ability to wait for whatever event somebody wants (via the lower level stream of events one can wait for as above), but we take away the method and thus the assumption that wait_for_in_block will Just Work under normal circumstances (which it occasionally won't, especially via the new APIs).

That said, if this did prove contentious I'd also be happy with us just adding a warning to the method that it may fail some times under normal operation.

[lexnv]

I still find this useful for instance on some scenarios such as the staking miner then one could select "best blocks" instead of "finalized block" but yeah it can be a footgun

Yep, this is still possible to do, although using a lower-level approach.
It might be possible that staking miner runs into a similar issue as detected by the CI, where it waits for the "BestBlock" but that might not (ever) be propagated to chainHead and cause a failure while using the RPCs methods with that block. Also, there's no guarantee that the current "BestBlock" will be finalized, since another "BestBlock { hash: None }" could indicate that the prior reported "BestBlock" is now pruned.

This is in response to #1236 just to keep the stronger guarantees around the RPC v2, waiting for @ascjones to confirm this is ok from their side 🙏

[niklasad1]

Yep, this is still possible to do, although using a lower-level approach.
It might be possible that staking miner runs into a similar issue as detected by the CI, where it waits for the "BestBlock" but that might not (ever) be propagated to chainHead and cause a failure while using the RPCs methods with that block. Also, there's no guarantee that the current "BestBlock" will be finalized, since another "BestBlock { hash: None }" could indicate that the prior reported "BestBlock" is now pruned.

Yepp, it's dangerous to use but "might be faster" than finalized blocks so in some applications it might be worth the risk (probably not)....

It's probably better for subxt just to support finalized blocks and for anything else one can implement custom logic for specific behavior but it's tricky to write such thing because the actual behavior when a subscription is closed or not. Not really clearly documented, see https://substrate.stackexchange.com/questions/7892/when-will-i-stop-receiving-transactionstatus-updates-for-a-transaction-submitted

[jsdw]

It's probably better for subxt just to support finalized blocks and for anything else one can implement custom logic for specific behavior but it's tricky to write such thing because the actual behavior when a subscription is closed or not.

In Subxt, if you use .next() on the TX progress events, rather than eg wait_for_in_block or whatever, then the stream will close itself at the right time and things anyway, so a naive impl doesn't need to worry about anything; it's just got a choice about whether it wants to report on any of the specific errors or just report a general "stream has closed without tx being in block" error :)