Skip to content

Conversation

@dasanra
Copy link
Contributor

@dasanra dasanra commented Sep 13, 2021

What it solves

Resolves #14

How this PR fixes it

Add new gas estimation endpoint and types

@dasanra dasanra requested a review from katspaugh September 13, 2021 13:13
@github-actions
Copy link

github-actions bot commented Sep 13, 2021

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

operation: 0,
})
console.log(result)
expect(result.safeTxGas).toBe('43663')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this deterministic?

Copy link
Contributor Author

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'),
Copy link
Member

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? 👀

Copy link
Contributor Author

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 😉

Copy link
Member

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.

Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 👍

@dasanra dasanra merged commit 5aa7e88 into main Sep 15, 2021
@dasanra dasanra deleted the feature/add-gas-estimation-endpoint branch September 15, 2021 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add gas estimation endpoint

3 participants