Replace token pool "validate tx" logic stubs with a new "activate" action #317
Conversation
The standalone "trackingId" parameter is being removed. Also differentiate between the stored TokenPool type in FireFly core and the simpler TokenPool data that is returned from the tokens plugin. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
awrichar
requested review from
nguyer,
nickgaski and
peterbroadhurst
as code owners
…tion A pool must be activated before using it. The activation step will trigger the pool creation event to be re-sent (so that it can be used to confirm the pool), and will also trigger the plugin to begin sending transfers from that specific pool. This solves a number of holes with token pool creation - including validating the pool announcement against the local plugin and ensuring transfers are not received before the pool is fully created. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Pools in "unknown" state will be activated and moved to "confirmed". 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>
Codecov Report
@@ Coverage Diff @@
## main #317 +/- ##
=======================================
Coverage 99.98% 99.98%
=======================================
Files 232 232
Lines 12578 12664 +86
=======================================
+ Hits 12576 12662 +86
Misses 1 1
Partials 1 1
Continue to review full report at Codecov.
|
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
A transaction ID is still required to trigger announcement, so the initial event back to the submitter needs to reflect the data passed into the connector. However, the later events triggered to all nodes technically do not need the transaction ID - so do not enforce this as a hard requirement. Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
peterbroadhurst
force-pushed
the
activate
branch
from
3a36ce1 to
8b88db9
Compare
peterbroadhurst
left a comment
peterbroadhurst
left a comment
There was a problem hiding this comment.
Spent a bunch of time staring at TokenPoolCreated() and didn't spot any concerns.
The only comment I had was on a method name.
I note the e2e tests are still failing after I updated the manifest and merged with main, so might be one more thing to look at before this clicks in.
| "github.com/hyperledger/firefly/pkg/tokens" | ||
| ) | ||
|
|
||
| func updatePool(storedPool *fftypes.TokenPool, chainPool *tokens.TokenPool) *fftypes.TokenPool { |
There was a problem hiding this comment.
I kept tripping over the update here reading the code.
Maybe tokenPoolToFFPool or newPoolFrom?
There was a problem hiding this comment.
Pushed a commit to use addPoolDetailsFromPlugin... it's a bit more verbose, but at least it's accurate.
Also removed the return value (switched it to only modify the input param) - so hopefully that makes it clearer as well.
|
I don't think the e2e failure is related - raising separate issue #328 |
|
kaleido-io#39 raised as a PR into this branch, to address the (unrelated) E2E failure |
|
(... the coverage red box will be addressed when #325 merges) |
Signed-off-by: Peter Broadhurst <peter.broadhurst@kaleido.io>
Signed-off-by: Andrew Richardson <andrew.richardson@kaleido.io>
3 participants
Part of #218
Resolves #315
Resolves #318
A pool must be activated before using it. The activation step will trigger the pool
creation event to be re-sent (so that it can be used to confirm the pool), and will
also trigger the plugin to begin sending transfers from that specific pool.
This solves a number of holes with token pool creation - including validating the
pool announcement against the local plugin and ensuring transfers are not received
before the pool is fully created.
Requires hyperledger/firefly-tokens-erc1155#43 (E2E tests will fail until this is integrated).
There are breaking changes in the internal API between core and the tokens connector,
but they should be invisible to an end user. Migrations are in place to transition any
pre-existing token pools.