Skip to content
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

feat: update dry run endpoint to use new runtime api call #1519

Merged
merged 5 commits into from
Oct 22, 2024
Merged

Conversation

filvecchiato
Copy link
Contributor

It updates the transaction dry run endpoint to accept a body with the transaction string, the sender address and the block number. it returns if the dry run was successful or if it errored and for which block hash

Copy link
Contributor

@IkerAlus IkerAlus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface, inputs and expected outputs look good to me. I haven't tested this properly though.

Also, remember to update the endpoint docs accordingly: https://paritytech.github.io/substrate-api-sidecar/dist/

@filvecchiato filvecchiato marked this pull request as ready for review October 17, 2024 13:36
@filvecchiato filvecchiato requested a review from a team as a code owner October 17, 2024 13:36
@filvecchiato filvecchiato linked an issue Oct 17, 2024 that may be closed by this pull request
Copy link
Member

@TarikGul TarikGul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks great, just a few questions.

@TarikGul
Copy link
Member

Great job :)

Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the docs that @IkerAlus mentioned, you need to update this file because the dry-run endpoint now accepts 2 additional parameters basides tx: senderAddress and at.
The usual steps I follow are :

  • I check the endpoint and schema sections in that file and make any necessary updates.
  • After I complete the changes, I verify that the yaml file has no errors by copy pasting it in the Swagger Editor.
  • If everything looks good, I run yarn build:docs
  • Then I open the page index.html (from folder docs/dist) in my browser to confirm that all changes have been applied.
  • If everything looks good, I push the changes.

@filvecchiato
Copy link
Contributor Author

I updated the docs and also updated the response to include response Type defining if its an error or a dispatch outcome

@@ -1067,7 +1067,7 @@ paths:
description: Use the dryrun call to practice submission of a transaction.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: Use the dryrun call to practice submission of a transaction.
description: Use the `dryRun` call to simulate the submission of a transaction without executing it so that you can check for potential errors and validate the expected outcome.

Comment on lines +4519 to +4528
type: object
properties:
actualWeight:
type: string
format: unsignedInteger
description: The actual weight of the transaction.
paysFee:
type: string
format: boolean
description: Whether the transaction pays a fee.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type: object
properties:
actualWeight:
type: string
format: unsignedInteger
description: The actual weight of the transaction.
paysFee:
type: string
format: boolean
description: Whether the transaction pays a fee.
oneOf:
- $ref: '#/components/schemas/TransactionDispatchOutcome'
- $ref: '#/components/schemas/TransactionValidityError'

@@ -4494,6 +4494,18 @@ components:
tx:
type: string
format: hex
DryRunBody:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
DryRunBody:
TransactionDispatchOutcome:
type: object
description: The result of a valid transaction submitted via the `dry-run` endpoint.
properties:
actualWeight:
type: string
format: unsignedInteger
description: The actual weight of the transaction.
paysFee:
type: string
format: boolean
description: Whether the transaction pays a fee.
TransactionValidityError:
type: object
description: The error result from an invalid transaction submitted via the `dry-run` endpoint.
properties:
errorType:
type: string
enum:
- Unimplemented
- VersionedConversionFailed
description: The type of transaction error, either `Unimplemented` or `VersionedConversionFailed`.
DryRunBody:

- `UnknownTransaction`: https://crates.parity.io/sp_runtime/transaction_validity/enum.UnknownTransaction.html
- `InvalidTransaction`: https://crates.parity.io/sp_runtime/transaction_validity/enum.InvalidTransaction.html
- `PostDispatchInfo`: https://docs.rs/frame-support/38.0.0/frame_support/dispatch/struct.PostDispatchInfo.html
- `Error Type`: https://paritytech.github.io/polkadot-sdk/master/xcm_runtime_apis/dry_run/enum.Error.html
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also get a DispatchError no ? Maybe we can add this here along with an additional schema (like TransactionDispatchOutcome and TransactionValidityError helper schemas) ?

Copy link
Contributor

@Imod7 Imod7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving since the changes in the docs I can add in a separate PR. Thank you!

@Imod7 Imod7 merged commit aeef4dc into master Oct 22, 2024
15 checks passed
@Imod7 Imod7 deleted the update_dry_run branch October 22, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update transaction dry run endpoint
4 participants