-
Notifications
You must be signed in to change notification settings - Fork 325
Opdata 3775 add functionality query decimals #4107
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: a3eece4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This is not really a good idea because we are trying to inject some contract specific logic into a generic piece of code. @mmcallister-cll @dskloetc Any ideas on how we should do this? Maybe put the decimal abi into input and have a result2 in output? |
Yeah, agree with @mxiao-cll, we shouldn't call this for every base function call, we can't guarantee every contract will have decimals My initial ideas are:
{
"endpoint": "function-with-decimals"
...
}
|
My preference is closest to this. But I would suggest a record/map where the results are returned under the same keys as the inputs. So the input could be something like:
And then the result would be of the shape:
But we could put anything else in However, I can also see how it would be beneficial to have the |
Contracts in RDD that do not support decimals() |
data: { | ||
description: 'Optional map of function calls', | ||
type: 'object' as unknown as Record<string, any>, | ||
required: false, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data seems to be very generic name, maybe additionalRequests
?
Also it is possible to type this correctly without using any
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggested data
because in the response these fields would also appear in data
.
additionalRequests
is also fine.
const { address, signature, inputParams, network } = param | ||
const { address, signature, inputParams, network, data } = param | ||
|
||
const mainResult = await this._executeFunction({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can run these two in parallel
} | ||
|
||
private async _processNestedDataRequest( | ||
data: Record<string, unknown> | undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be typed as Record<string, RequestParams>
): Promise<Record<string, any>> { | ||
if (!data || typeof data !== 'object') return {} | ||
|
||
const results: Record<string, any> = {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we not use any here?
resultField: req.resultField, | ||
} | ||
|
||
const subRes = await this._executeFunction(nestedParam) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we run these in parallel?
expect(response.json()).toMatchSnapshot() | ||
}) | ||
|
||
it('should return success with additional data requests ', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated test case?
Closes #OPDATA 3775 - Step 1
Description
Add functionality to query decimals from contract and return it as part of the result
Steps to Test
yarn test packages/source/view-function-multi-chain/test
Input
Output
Quality Assurance
infra-k8s
configuration file.adapter-secrets
configuration file or update the soak testing blacklist.test-payload.json
file with relevant requests.feature/x
,chore/x
,release/x
,hotfix/x
,fix/x
) or is created from Jira.