services/horizon: add option to submit without waiting for transaction final state #3564
Conversation
|
@bartekn I'd like to propose this change as an experimental feature of Horizon that is only enabled if explicitly indicated. Is there a common way to flag and enable/disable experimental features in Horizon? Or should I just add a new config option to the top-level CLI? |
|
|
||
| return asBool, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Hmm, we already have ?include_failed=true/false; there must be code parsing that out somewhere 🤔
There was a problem hiding this comment.
I could only find a GetString function. It's possible I missed it. Thanks for pointing it out. This could be replaced with any existing function if there is one.
There was a problem hiding this comment.
Ah, we use a library for this I guess (see helpers.go::getParams():403) which is catered towards HTTP-style URLs.
| result := handler.Submitter.Submitter.Submit(r.Context(), info.raw) | ||
| return handler.responseAsync(r, info, result) | ||
| } | ||
|
|
||
| submission := handler.Submitter.Submit( |
There was a problem hiding this comment.
From my understanding, handler.Submitter.Submit (i.e. System::Submit()) will do interaction with the database, checking if the tx exists already, etc. then block until the submission is processed from the queue, all without accessing the lower-level submitter (i.e. it uses System's own APIs). OTOH, handler.Submitter.Submitter.Submit is the raw way to interact with Core and that's what you're doing here.
(the above is just a summary of the PR, for myself as notes and to confirm my understanding)
A few questions:
-
Are we concerned at all about the lack of extra checks (e.g.
checkTxAlreadyExistsor helpers with handlingbadSeq) or metrics for this thatSystemprovides, or is the intent to let API consumers be true "power users"? If the latter (which I like in principle quite a bit), I'd love a long-term goal where this is actually the recommended way to submit txs while the existing way is more of a?babysit=truefor people who don't want to think too hard about txsub. Either way, maybe we want anotherSystemthat does an async version of everything for an "official" implementation. -
This PR doesn't introduce any DB changes, and I'm wondering if we would want that (e.g. something like a
inprogress=true/falsecolumn in thehistory_transactionstable). Probably not, but I ask because... -
It's unclear to me what the recommended way to check transaction status is with this endpoint. I assume it's "tx is pending until
GET /transactions/<hash>doesn't 404". At that point, thesuccessfulfield is sufficient for the trivial success/failure case, and grokking theresult_xdrshould give specifics. Is that a fair assumption?
|
GH autoclosed this because I removed an old release branch. Unfortunately I can't reopen. Can you reopen against |
|
@bartekn GitHub does not allow me to reopen the PR, or change its base branch. I believe someone else may be taking on turning this feature request and spike into a full fledged feature. If that happens and they'd like to use this code, they can checkout the code from this PR. |
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Add option to submit without waiting for transaction final state. Transactions submitted with the additional parameter
async=trueare submitted directly to the submitter (stellar-core) and a pending transaction response is returned with a HTTP status code 202 Accepted.Why
Applications do not always want to keep a http connection open for ~5 seconds to find out the final state of the submitted transaction. In some cases applications would prefer to submit and lookup the transaction later to see if it was successful, failed, or just not included.
Technically if an application was to cover all edge cases it would need to support that later lookup anyway, because it is possible for the Horizon to timeout, or the connection to be interrupted. It's also always possible for a submitted transaction to be scooped up by some other party if the transaction has been broadcasted or shared, and for that other party to resubmit the transaction during its time bound window of validity, even if Horizon fails the transaction immediately on the initial submit.
For applications that implement these edge cases already, submitting without waiting may be attractive.
Known limitations
This PR includes no tests for the new behavior of the endpoint. I've been working on a test, but it's a work-in-progress, and I'm learning a bit about how action tests work so it's taking me a bit longer.