Skip to content
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

feat: Add estimate_gas and get_gas_price calls #34

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

zvolin
Copy link
Member

@zvolin zvolin commented Aug 17, 2023

No description provided.

Copy link
Member

@roberts-pumpurs roberts-pumpurs left a comment

Choose a reason for hiding this comment

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

Mostly lgtm. Added some comments for provoking some thoughts/spawning discussions, but none are really actionable. Approving because I won't have the time for another review today

src/ethereum-canister/candid.did Show resolved Hide resolved
src/ethereum-canister/candid.did Outdated Show resolved Hide resolved
src/ethereum-canister/candid.did Show resolved Hide resolved
src/ethereum-canister/src/lib.rs Show resolved Hide resolved
src/interface/src/network.rs Show resolved Hide resolved
@roberts-pumpurs
Copy link
Member

I am not entirely convinced by the general architecture that your ethereum-canister would need to expose and proxy different contract methods (e.g erc20 balanceOf, ownerOf, etc). Because this means that are putting on the mantel of supporting all the different ERCs that are going to come out.
A much more scalable approach imo would be just to expose generic "query" and simulator methods, and then there can be a specific etherc20-canister that proxies individual exact calls with .did nice interface. This also would allow the community to build the wrappers for whichever contract they want to interact with. Most fun projects out there in the wild are not generic erc20/erc721 contracts. But that's just food for thought 😄

@oblique
Copy link
Member

oblique commented Aug 18, 2023

I am not entirely convinced by the general architecture that your ethereum-canister would need to expose and proxy different contract methods (e.g erc20 balanceOf, ownerOf, etc).
....
A much more scalable approach imo would be just to expose generic "query"

This is a requirement, but we may expose a raw call API.

@zvolin zvolin merged commit e6f6ca9 into eigerco:main Aug 18, 2023
3 checks passed
@zvolin zvolin deleted the feat/estimate-gas branch August 18, 2023 10:28
@zvolin zvolin mentioned this pull request Aug 18, 2023
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.

3 participants