-
Notifications
You must be signed in to change notification settings - Fork 23
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
FLIP 316: FCL Ethereum Provider for Cross-VM Apps #317
base: main
Are you sure you want to change the base?
Conversation
c8abcaf
to
5d9c097
Compare
gasLimit: gasLimit, | ||
value: valueBalance | ||
) | ||
assert(callResult.status == EVM.Status.successful, message: "Call failed") |
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 think here also fail case should be ok
|
||
/// Executes the calldata from the signer's COA | ||
/// | ||
transaction(evmContractAddressHex: String, calldata: String, gasLimit: UInt64, value: UFix64) { |
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'm wondering if the value
field may need to be UInt
for consistency with EVM patterns (which would require updates to the subsequent code). Is the conversion is handled by fcl before passing as a transaction 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.
yeah UInt256 is better choice there
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.
Sounds good - I copied this script from https://github.com/onflow/flow-evm-bridge/blob/main/cadence/transactions/evm/call.cdc.
I'll update the script here, but I can also make a PR to https://github.com/onflow/flow-evm-bridge, if we feel this is a better standard to use in general @bluesign @sisyphusSmiling ?
Co-authored-by: Giovanni Sanchez <[email protected]>
( I don't know much of EVM wallet flow, may be totally wrong here. ) If I understood correctly, eth provider is injected after FCL login. But I think injecting provider before can be better option. The flow I see on kitty punch, If I click login, it is popping up me a wallet selection dialog. Now if I add updated FCL I have to first login user with non-evm Flow Wallet to have provider in this list. ( I may be wrong on this assumption, but it seems provider is created on current user ) I think it may be better, if provider is in this list ( as we detect in discovery ) let's say BlueWallet ( hypothetical non-erm flow wallet extension ) and another provider maybe something like ( Other Flow Wallets ) which redirects to discovery. So when I click BlueWallet it can directly authenticate with FCL ( extension pop up ) and I can use it transparently. ( via just initializing FCL ) |
Thank you for the feedback @bluesign and I can clarify a bit on the timing of the provider creation.
Apologies if it's not clear in the proposal, but the intended use is actually as you describe: the provider is supposed constructed before the user has authenticated with FCL. So for the KittyPunch/"Blue Wallet" example, it would be as follows:
I'll see if I can make some diagrams & add a better description to make this more clear. |
thanks @jribbink for this is great explanation. It all makes sense now, I think this is a great idea and very useful for filling the current gap. |
Closes #316