-
Notifications
You must be signed in to change notification settings - Fork 38
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
[JS->TS] Migrate invokeViewFunction.js #815
Conversation
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.
This one is tricky in TypeScript since you need to invoke a method by name with parameters at runtime. This means TypeScript can't really properly check that the use of the exchange
type is valid since it won't know what method is being called until runtime.
c747b04
to
e0d5563
Compare
scripts/invoke_view_function.ts
Outdated
} | ||
const [functionName, ...arg] = args; | ||
const exchange = await getBatchExchange(artifacts); | ||
const info = await (exchange as any)[functionName].call(...arg); |
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.
Sadly... This was unsuccessful.
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.
Hmm, not sure then
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.
info is of type BN which doesn't seem to print well with log.info (it works with console.log)
If nobody is opposed, I'd like to take the opportunity to remove this function that is not used anywhere. However, in the essence of making everything TS compatible, I would endure and see this through if desired. |
scripts/invoke_view_function.ts
Outdated
} | ||
const [functionName, ...arg] = args; | ||
const exchange = await getBatchExchange(artifacts); | ||
const info = await (exchange as any)[functionName].call(...arg); |
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.
info is of type BN which doesn't seem to print well with log.info (it works with console.log)
// one would usually grep for the results of this execution. Thus, | ||
// the raw output is printed rather than include irrelevant log details. | ||
// eslint-disable-next-line no-console | ||
console.log(info); |
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.
Curious, is there a reason you didn't use info.toString()
?
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.
Actually I think I meant to although this printed fine with getCurrentBatchId
Partially fulfills #330. Unfortunately, I have not been able to get expected values returned from this script just yet. This snippet is the closest example in the current code base
dex-contracts/src/onchain_reading.ts
Lines 89 to 91 in 7cabfe1
I can verify that logging
contract.methods
does indeed contain the methods as strings.Test Plan
npx truffle exec build/common/scripts/invoke_view_function.js --network mainnet getCurrentBatchId
Currently getting back
undefined
.