Add special case FungibleToken handler #46
Conversation
sisyphusSmiling
force-pushed
the
add-special-case-handler
branch
from
031e03b to
d2c8366
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
==========================================
+ Coverage 82.88% 85.34% +2.45%
==========================================
Files 18 18
Lines 818 812 -6
==========================================
+ Hits 678 693 +15
+ Misses 140 119 -21 ☔ View full report in Codecov by Sentry. |
| /// | ||
| /// This contract defines the interfaces for the FlowEVM Bridge Handlers. These Handlers are intended to encapsulate | ||
| /// the logic for bridging edge case assets between Cadence and EVM and require configuration by the bridge account to | ||
| /// enable. Contracts implementing these resources should be deployed to the bridge account so that privileged methods, |
There was a problem hiding this comment.
It sounds to me like you're saying here that a contract like FiatToken would have to be deployed to the bridge account to be compatible. Is that true? I don't think that will be possible since these are already deployed to other accounts
There was a problem hiding this comment.
Good question. No, the existence of handler assumes the contract is not deployed to the bridge. Rather, what I mean by Contracts implementing these resources should be deployed to the bridge account is that contracts implementing the *Handler* interfaces should be deployed to the bridge. Concretely, FlowEVMBridgeHandlers is one such contract.
| /// The Minter enabling minting of Cadence tokens on fulfillment from EVM | ||
| access(self) let minter: @{FlowEVMBridgeHandlerInterfaces.TokenMinter} | ||
|
|
||
| init(targetType: Type, targetEVMAddress: EVM.EVMAddress?, minter: @{FlowEVMBridgeHandlerInterfaces.TokenMinter}) { |
There was a problem hiding this comment.
I think that it would be better if the minter resource could just be sent via a public function in a public capability, like addMinterCapability() and it would check to see that the type and such are correct so that people can't sent junk minters to it. That way, the minter can be sent trustlessly, which I think will be important for some of these projects
There was a problem hiding this comment.
That's a good idea. So do something like add an expectedMinterType on init instead of the minter itself and confirm that the minter added in addMinterCapability is the expected type?
Alternatively, we could entitle the addMinterCapability and publish a private Capability to the minter provider so they can privately insert the minter. It closes the loop a bit more but does require a bit more coordination.
I like that either option gives us the ability to reserve a handler for a given type without first needing a minter resource.
There was a problem hiding this comment.
I like the option that requires the least coordination, so I think just it being a public function that checks the minter type is sufficient
There was a problem hiding this comment.
Updated to address in 3f5d19e
|
|
||
| /// Minter interface for configurations requiring the minting of Cadence fungible tokens | ||
| /// | ||
| access(all) resource interface TokenMinter { |
There was a problem hiding this comment.
So in a contract that wants to be supported here, all they would have to do is have their minter resource implement this interface?
If we expect this implementation to happen as part of the cadence 1.0 upgrade, we need to deploy this interface on all the networks before then so the migrations work properly
There was a problem hiding this comment.
Yeah, I didn't see a way around it if we want to store minters with specific interface guarantees. My thought was that we would deploy this contract after the migration and any implementers would then update their contracts, but that feels cumbersome...
Maybe we could deploy this contract with just the TokenMinter interface before the migration so that the contract could be migrated and included in dependent contract updates. And ofc stage this contract which shouldn't be a problem since all of its dependencies will also be migrated. Think that would work?
Edit: I realized that's basically what you suggested. Ha alright in that case I'll just make an 0.42 version as well
There was a problem hiding this comment.
yep, that sounds good to me
Closes: #26
Stacked on: #39
For contributor use:
masterbranchFiles changedin the Github PR explorer