fix: async approval add polling

[rukmini-basu-da]

Adds a polling mechanism and a new status awaiting-signature:

1.pending: prepared command not sent anywhere
2.awaiting-signature: sent to signing provider but haven't received signature yet (this happens after we hit approve the first time)
3. signed: found signature on signing provider (so as soon as we go to this activity on the ui, it starts querying the signing driver for this tx if it's in awaiting-signature and then as soon as it finds it we update the sqllite status)
4. executed: happens after we hit approve the second timeand happy path if the execute against the ledger is successful
5. failed: as is, can happen if something goes wrong at any step but would also include if something fails on the execute step

[pawelstepien-da]

This is used by signing worker and I think it could break something there in condition that compares refreshedTx.status !== 'pending', or at least result in unnecessary processing txes that are not pending-signature. That module will probably need to adjust to new status.

[rukmini-basu-da]

I've modified the signingWorker to now look at refreshedTx.status !== 'pending' && refreshedTx.status !== 'awaiting-signature' and modified the signAndExecute (updated the doc at the top to explain the lifecycle/what happens on each tick)

[alexmatson-da]

whats the idea behind expectedStatus / how will it be used?

[rukmini-basu-da]

it's currently used in the applySigningResult

            const applied = await this.store.setTransactionSigned(
                tx.id,
                now,
                signingResult.txId,
                { expectedStatus: tx.status }
            )

            if (!applied) {
                const current = await this.store.getTransaction(tx.id)
                return { status: current!.status }
            }

and I think this was when I was trying tackle the scenario where someone hits approve twice on the execute where the version approve click has already gone through the sign/execute flow but then another one is in flight so this would prevent the setTransactionSigned row from matching (since the status would be executed) and then the UI shows the transaction as completed

[alexmatson-da]

maybe you can have it wait until its no longer pending?

[alexmatson-da]

why status pending and not awaiting-signature?

[rukmini-basu-da]

yeah, this is a bit weird because this value goes on as SignResult and the api spec has SignResultPending (where the status is a const: pending), so this would fail the schema validation. Kinda related to #2381 (comment)

I eventually wanted to add the SigningResultAwaitingSignature but figured that could happen in a follow up pr, but maybe it makes sense to add now. wdyt?

[alexmatson-da]

I dont quite see a reason to not send awaiting-signature to the dapp as well, but maybe add a TODO + tech debt issue for now

[alexmatson-da]

why was this removed?

[rukmini-basu-da]

Moved the path to polling the provider for the signature to refreshTransaction. And also, signWithDriver is only called from sign now and sign calls loadPreparedTransactionForSigning first so any tx with an externalTxId throws before signWithDriver runs so the if(tx.externalTxId) would never be reachable

[alexmatson-da]

do we need to handle the return value? I can't tell if the if statement should check signingResult.status or appliedSigningResult.status (or if it matters)?

[rukmini-basu-da]

good call, this guard should help if another request tries to sign or execute the same transaction concurrently (which ideally should be prevented by loadPreparedTransactionForSigning), but it's worth having to make sure the applied result and driver result agree on the status