-
-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
local-tx-submission extension: evaluation of transaction execution units #176
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It'll now be used also for evaluating transactions, so the name would've been misleading.
Thinking through this a bit, I concluded that the existing local-tx-submission was the most suitable place for adding this functionality. Users are already submitting serialized transactions through that protocol, and in essence, evaluating execution units shouldn't be much different. So, this _extends_ existing Ogmios' capabilities and protocol and deviates now from the the core local-tx-submission protocol. From a client's perspective, this remains quite transparent and would look like just another method usable through Ogmios' websocket. Behind the scene, it'll actually be mostly implemented using the state-query protocol as we need to (a) resolve the transaction inputs into a UTXO set and (b) lookup current protocol parameters to correctly evaluate the transaction.
Remains now to run the needed state-query queries to get the necessary protocol parameters, utxo and time interpreter needed to calculate execution units.
This effectively now starts two connections PER websocket connections... I might want to revisit that later if it turns out to cause too much overhead; There's already another connection started for the health check (although, one per Ogmios' server only) and it may be possible to wire into that one instead. However, since all protocols on that connection are pretty much idling all the time, it should cause any problem.
KtorZ
force-pushed
the
KtorZ/172/evaluate-execution-units
branch
from
February 12, 2022 18:13
fdcf28b
to
4bd5b5e
Compare
KtorZ
force-pushed
the
KtorZ/172/evaluate-execution-units
branch
from
February 12, 2022 18:31
4bd5b5e
to
ba00e78
Compare
Still not perfect on CBOR deserialisation failures. Somehow, the decoder always returns a byteoffset of 0 and a 'expected tag' error. This is most probably a problem with the upstream CBOR decoder in the ledger... I will investigate one day to see if I can get anything better there.
Without this, sending invalid Tx-Monitor errors which simply return a generic client fault without much information.
This enables e2e tests on real Testnet, which adds confidence that things are working properly. So far, only basic contracts and executino flow but I'll generate some slightly more complex test scenarios in upcoming commits.
KtorZ
force-pushed
the
KtorZ/172/evaluate-execution-units
branch
from
February 13, 2022 12:08
f8908e7
to
1939ae5
Compare
This is so-to-speak a breaking change BUT, the API still supports the old way of submitting transactions which will not yield the transaction id. Indeed, the request has also been changed from ```json >>> {"type":"jsonwsp/request","version":"1.0","servicename":"ogmios","methodname":"SubmitTx","args":{"bytes":"84a50081[...]c40ef5f6"}} { "type": "jsonwsp/response", "version": "1.0", "servicename": "ogmios", "methodname": "SubmitTx", "result": "SubmitSuccess", "reflection": null } ``` to ```json >>> {"type":"jsonwsp/request","version":"1.0","servicename":"ogmios","methodname":"SubmitTx","args":{"submit":"84a50081[..]900ff5f6"}} { "type": "jsonwsp/response", "version": "1.0", "servicename": "ogmios", "methodname": "SubmitTx", "result": { "SubmitSuccess": { "txId": "182ce8ad170adf9ad040b94053822f92172a518f442f17a9f0fbe32972b7f8d7" } }, "reflection": null } ``` The former will yield a response as before whereas the later will yields the new SubmitTxResponse which also contains a transaction id.
KtorZ
force-pushed
the
KtorZ/172/evaluate-execution-units
branch
from
February 13, 2022 13:39
d87679d
to
21590f5
Compare
Whoops. I did actually spent the collateral during a smoke test... so the transaction no longer valid 😬 It'd be nice to allow Ogmios to also take an extra UTXO as input during evaluation to allow a more development-friendly mode where transaction may refer to inputs which do not exist (yet) on the network. This would make it a lot easier to construct test cases and to toy around.
KtorZ
force-pushed
the
KtorZ/172/evaluate-execution-units
branch
from
February 13, 2022 14:27
4b5f3b9
to
b5ff588
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #172 & #174
📍 Rename 'SubmitTxPayload' into 'SerializedTx'
It'll now be used also for evaluating transactions, so the name would've been misleading.
📍 Draft tx-submission protocol extension.
Thinking through this a bit, I concluded that the existing local-tx-submission was the most suitable place for adding this functionality. Users are already submitting serialized transactions through that protocol, and in essence, evaluating execution units shouldn't be much different.
So, this extends existing Ogmios' capabilities and protocol and deviates now from the the core local-tx-submission protocol. From a client's perspective, this remains quite transparent and would look like just another method usable through Ogmios' websocket. Behind the scene, it'll actually be mostly implemented using the state-query protocol as we need to (a) resolve the transaction inputs into a UTXO set and (b) lookup current protocol parameters to correctly evaluate the transaction.
📍 Write (& document) JSON encoders for ScriptFailure.
📍 Add 'Array' utilities to Ogmios' Prelude.
📍 Draft execution units evaluation.
Remains now to run the needed state-query queries to get the necessary protocol parameters, utxo and time interpreter needed to calculate execution units.
📍 Implement effectful execution units evaluator.
📍 Wire the execution units evaluator through websocket connection stack.
This effectively now starts two connections PER websocket connections... I might want to revisit that later if it turns out to cause too much overhead; There's already another connection started for the health check (although, one per Ogmios' server only) and it may be possible to wire into that one instead.
However, since all protocols on that connection are pretty much idling all the time, it should cause any problem.
📍 Add JSON-schema for the new 'EvaluateTx' local-tx-submission message.
📍 Improve on error reporting for the SubmitTx and EvaluateTx errors.
Still not perfect on CBOR deserialisation failures. Somehow, the decoder always returns a byteoffset of 0 and a 'expected tag' error. This is most probably a problem with the upstream CBOR decoder in the ledger... I will investigate one day to see if I can get anything better there.
📍 Add missing matcher for Tx-Monitor in top-level error handler.
Without this, sending invalid Tx-Monitor errors which simply return a generic client fault without much information.
📍 Add support for 'EvaluateTx' in the TypeScript client & repl
This enables e2e tests on real Testnet, which adds confidence that
things are working properly. So far, only basic contracts and
executino flow but I'll generate some slightly more complex test
scenarios in upcoming commits.
📍 Add submitted transaction's id to the success response of 'SubmitTx'
This is so-to-speak a breaking change BUT, the API still supports the old way of submitting transactions which will not yield the transaction id. Indeed, the request has also been changed from
to
The former will yield a response as before whereas the later will yields the new SubmitTxResponse which also contains a transaction id.
📍 Add JSON-schema property tests (and fixes) for EvaluateTx
📍 Replace e2e golden transaction in TypeScript evaluateTx tests
Whoops. I did actually spent the collateral during a smoke test... so the transaction no longer valid 😬
It'd be nice to allow Ogmios to also take an extra UTXO as input during evaluation to allow a more development-friendly mode where transaction may refer to inputs which do not exist (yet) on the network. This would make it a lot easier to construct test cases and to toy around.
📍 Reword tx-submission user-guide and add section about EvaluateTx.
📍 Fill-in CHANGELOG for 5.2.0