feat: add submit and wait for transaction submission #528
Conversation
Co-authored-by: Jackson Mills <aim4math@gmail.com>
Co-authored-by: Jackson Mills <aim4math@gmail.com>
| payment_transaction, WALLET, client | ||
| ) | ||
| await accept_ledger_async(delay=1) | ||
| payment_transaction_signed_blob = encode(payment_transaction_signed.to_xrpl()) |
There was a problem hiding this comment.
This is perhaps a separate ticket, but encode(tx.to_xrpl()) feels like a good helper function on the Transaction object (something like tx.blob()
There was a problem hiding this comment.
I just added the helper function for this ticket.
| if transaction.is_signed(): | ||
| return transaction | ||
|
|
||
| if not wallet: | ||
| raise XRPLException( | ||
| "Wallet must be provided when submitting an unsigned transaction" | ||
| ) |
There was a problem hiding this comment.
nit: I'd recommend including the else even though it's not strictly necessary because the logic relies on that return to make sense. Each time I've reviewed the PR it's taken me a minute or two to remember why it's guaranteed that at this point the transaction MUST require a signature, and I think adding this else would clarify that.
| if transaction.is_signed(): | |
| return transaction | |
| if not wallet: | |
| raise XRPLException( | |
| "Wallet must be provided when submitting an unsigned transaction" | |
| ) | |
| if transaction.is_signed(): | |
| return transaction | |
| else: | |
| if not wallet: | |
| raise XRPLException( | |
| "Wallet must be provided when submitting an unsigned transaction" | |
| ) |
There was a problem hiding this comment.
I think adding a comment saying "Attempts to validate fee, autofill, and sign the transaction when unsigned" would suffice. The return statement already tells that we only proceed to the remaining steps for unsigned transaction and I would prefer not to have many conditional layers in the code. Other functions also add comments when if/else clarification is needed.
| signed_transaction = await _get_signed_tx( | ||
| transaction, client, wallet, check_fee, autofill | ||
| ) | ||
| return await send_reliable_submission( |
There was a problem hiding this comment.
Note for later: If we make send_reliable_submission deprecated, we should move that logic into this function, or a helper.
There was a problem hiding this comment.
Agree, we can mention that in the deprecate ticket.
High Level Overview of Change
Currently, user has to call 2 functions (
autofill_and_signandsend_reliable_submission) in order to sign, autofill, submit and verifies that the transaction has been included in a validated ledger. This PR would simplify the workflow to a single call (submit_and_wait) similar to xrpl.js (the old workflow would still work so it is not a breaking change). Tests and snippets are also fixed to use the new method.Couple of other improvements:
fail_hardis_signedchecks for multi-sign transactionType of Change