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: add optional arguments for block height and block time to Mutation.dryRun #2207

Closed
netrome opened this issue Sep 17, 2024 · 2 comments
Closed
Labels
graphql-api Affects API of the GraphQL

Comments

@netrome
Copy link
Contributor

netrome commented Sep 17, 2024

Context

Currently, the dryRun mutation looks like this:

	"""
	Execute a dry-run of multiple transactions using a fork of current state, no changes are committed.
	"""
	dryRun(txs: [HexString!]!, utxoValidation: Boolean, gasPrice: U64): [DryRunTransactionExecutionStatus!]!

While the block producer internally supports providing a block height and a block time (after #2206 is merged). We should add options for these parameters in the graphql API, and in the graphql client implementation.

Note that adding optional arguments to a graphql API is a non-breaking change, but if we modify the graphql client dry_run_opt function, it's a breaking change that needs to be coordinated with the Rust SDK team.

Definition of Done

The GraphQL API mutation Mutation.dryRun supports optional arguments for specifying block height and block time.

@netrome netrome added the graphql-api Affects API of the GraphQL label Sep 17, 2024
@xgreenx
Copy link
Collaborator

xgreenx commented Sep 17, 2024

We already have an issue for it #1749

But the customization of the block height is a bad idea because if you try to create a block in the future, the executor will fail(the previous block doesn't exist). If you try to create block in the past, then you need to use storage with the state in the past(#2062). But it is really heavy operation and I'm not sure do we want to allow it in public nodes or not

@netrome
Copy link
Contributor Author

netrome commented Sep 17, 2024

We already have an issue for it #1749

Alright, I'll include this info in #1749 then.

But the customization of the block height is a bad idea because if you try to create a block in the future, the executor will fail(the previous block doesn't exist). If you try to create block in the past, then you need to use storage with the state in the past(#2062).

Interesting. Is this the desired behavior? It seems to me like a contract developer could benefit from the ability of dry running transactions against future block heights. For example if you time-lock some functionality it would be nice to be able to test this time lock by dry-running a transaction in a future block.

But it is really heavy operation and I'm not sure do we want to allow it in public nodes or not.

Yeah should we even expose any type of dry running functionality for public nodes? That just feels like a DoS vulnerability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
graphql-api Affects API of the GraphQL
Projects
None yet
Development

No branches or pull requests

2 participants