-
Notifications
You must be signed in to change notification settings - Fork 26
Add gas estimation endpoint #19
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
Conversation
ESLint Summary View Full Report
Report generated by eslint-plus-action |
| operation: 0, | ||
| }) | ||
| console.log(result) | ||
| expect(result.safeTxGas).toBe('43663') |
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.
Is this deterministic?
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.
While doing the exact same operation tt should be deterministic at least until the smart contract is updated
| describe('callEndpoint', () => { | ||
| it('should accept just a path', async () => { | ||
| await expect( | ||
| callEndpoint('https://safe-client.xdai.staging.gnosisdev.com/v1', '/balances/supported-fiat-codes'), |
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.
Wow, how did this work before? 👀
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.
While the not unified version is online this would work, but, as we already switched to the unified version better to update this just in case 😉
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.
Ah, because it's not TypeScript. It didn't detect that this endpoint doesn't exist.
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.
Nice! 👍
What it solves
Resolves #14
How this PR fixes it
Add new gas estimation endpoint and types