add ink v6 contract dry run#1740
Conversation
|
Thank you for the PR, I promise to get to this sometime in the next 2 days! |
andrew-ifrita
left a comment
andrew-ifrita
left a comment
There was a problem hiding this comment.
Sorry for the long delay here. I have a handful of comments and concerns that need to be addressed before this can be approved and merged.
Aside from the specific points mentioned in the code
- The title mentions "ink! v6" but there is nothing version specific here. The PR description might just need a little more context.
- There are no tests for the new endpoint. This is a hard blocker.
- There are no docs for the new endpoint. This is another hard blocker
| * @param _req | ||
| * @param res | ||
| */ | ||
| private dryRun: IPostRequestHandler<IBodyContractMetadata, IContractDryParams> = async ( |
There was a problem hiding this comment.
Here we have IPostRequestHandler but it is mounted as a GET handler with
this.safeMountAsyncGetHandlers([['/dry-run', this.dryRun as RequestHandler]]);| protected initRoutes(): void { | ||
| this.router.use(this.path, validateAddress); | ||
| this.safeMountAsyncPostHandlers([['/query', this.callContractQuery as RequestHandler]]); | ||
| this.safeMountAsyncGetHandlers([['/dry-run', this.dryRun as RequestHandler]]); |
There was a problem hiding this comment.
But this is returning IPostRequestHandler while being mounted as a GET handler.
Which one is it?
| { params: { address }, query: { caller, payValue = '0', inputData } }, | ||
| res, | ||
| ): Promise<void> => { | ||
| const dryRunResult = await this.api.call.reviveApi.call( |
There was a problem hiding this comment.
This needs a check if reviveApi is available as well as graceful handling if it is not.
| caller: string; | ||
| payValue: string; | ||
| inputData: string; |
There was a problem hiding this comment.
I don't see any of these values being validated anywhere which can lead to some bugs and poor UX / hard to debug issues.
|
|
||
| export interface IContractDryParams extends Query { | ||
| caller: string; | ||
| payValue: string; |
There was a problem hiding this comment.
I think this is meant to be optional, based on the default values that is has above.
| }; | ||
|
|
||
| /** | ||
| * Send a message call to a dry run contract. It defaults to get if nothing is inputted. |
There was a problem hiding this comment.
It defaults to get if nothing is inputted.
Personally this is a bit hard to understand on a first read through. Maybe something like
Execute a dry run contract call without requiring the full ABI
is better?
|
@BurnWW Any update here or shall we consider this stale? |
|
Closing as stale. Feel free to reopen if/when the above comments can be addressed. |
3 participants
The current contract interface requires transferring a large ABI file with each call. I am now submitting a new dry api for more lightweight contract queries.