Add ability to bind a contract interface to a token pool #1123
Conversation
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Codecov Report
@@ Coverage Diff @@
## main #1123 +/- ##
=======================================
Coverage 99.34% 99.34%
=======================================
Files 303 303
Lines 19695 19789 +94
=======================================
+ Hits 19566 19660 +94
Misses 120 120
Partials 9 9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
awrichar
requested review from
peterbroadhurst,
nguyer,
shorsher and
nickgaski
as code owners
internal/tokens/fftokens/fftokens.go
Outdated
| type tokenInterface struct { | ||
| Format core.TokenInterfaceFormat `json:"format"` | ||
| ABI []*abi.Entry `json:"abi,omitempty"` | ||
| Methods []*fftypes.FFIMethod `json:"methods,omitempty"` |
There was a problem hiding this comment.
I think ffi would be less confusing for the name of this parameter if possible.
As on the other end of the token connector where it talks to EVMConnect/EthConnect etc. the methods array is going to represent an array of ABI-style methods. Also where abi is used above, I think it's actually an array of ABI methods (not the whole ABI).
| @@ -643,13 +662,68 @@ func (ft *FFTokens) ActivateTokenPool(ctx context.Context, nsOpID string, pool * | |||
| return false, nil | |||
| } | |||
|
|
|||
| func (ft *FFTokens) MintTokens(ctx context.Context, nsOpID string, poolLocator string, mint *core.TokenTransfer) error { | |||
| func (ft *FFTokens) prepareABI(ctx context.Context, methods []*fftypes.FFIMethod) ([]*abi.Entry, error) { | |||
| abiMethods := make([]*abi.Entry, len(methods)) | |||
There was a problem hiding this comment.
This line confirms for me the comment earlier - that the abi field of the payload passed to the Token connector is not the full ABI, but rather the array of ABI-formatted Methods.
... all to say, I feel quite strongly we need to rename methods to ffi in the payload to the token connector (or rename both to abiMethods/ffiMethods)
There was a problem hiding this comment.
Or should it be called methods in both cases (and the format param tells you how to decode it)?
There was a problem hiding this comment.
Since it's not really an abi or ffi, it's always a list of methods encoded in abi or ffi format.
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Part of FIR-16.
Depends on hyperledger/firefly-tokens-erc20-erc721#106.
Discussion about the overall architecture/usefulness of this proposed feature should be directed to FIR-16, while discussion here should focus on specific technical details of the current proposed approach.
This adds the ability for FireFly to provide ABI details to a token connector, making it easier for the token connector to know the shape of the underlying contract it is dealing with (vs. having to probe/guess at the methods available).
Notes
config.address. Worth mentioning that FireFly doesn't really "know" you can create pools against pre-deployed contracts (asconfigis opaque), but it's up to the user to understand the combination of features that are useful here.interfaceon every pool creation even while relying on the factory sitting on the other side of the token connector to do the deploy. Seems like a less useful combo, but calling it out here for completeness.