feat(nano): emit a token-creation event including for nano executions#1494
feat(nano): emit a token-creation event including for nano executions#1494
Conversation
|
| Branch | feat/nano/emit-token-creation-event |
| Testbed | ubuntu-22.04 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result minutes (m) (Result Δ%) | Lower Boundary minutes (m) (Limit %) | Upper Boundary minutes (m) (Limit %) |
|---|---|---|---|---|
| sync-v2 (up to 20000 blocks) | 📈 view plot 🚷 view threshold | 1.69 m(-2.18%)Baseline: 1.73 m | 1.56 m (92.01%) | 2.08 m (81.51%) |
9d293c8 to
51ac5df
Compare
51ac5df to
f5b90d6
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1494 +/- ##
==========================================
+ Coverage 86.15% 86.20% +0.04%
==========================================
Files 440 440
Lines 33993 34055 +62
Branches 5315 5326 +11
==========================================
+ Hits 29288 29358 +70
+ Misses 3677 3673 -4
+ Partials 1028 1024 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hathor/event/event_manager.py
Outdated
| """Calculate the amount minted by a token-creation transaction.""" | ||
| assert tx.hash is not None | ||
| total = 0 | ||
| for tx_output in tx.outputs: |
There was a problem hiding this comment.
Since it can have a NanoHeader, what about deposits, transferring token authority to the contract and internal minting?
There was a problem hiding this comment.
Could those affect the initial amount when a token is created?
There was a problem hiding this comment.
I assumed they couldn't, and tracking changes to the token is outside of the scope of this event.
There was a problem hiding this comment.
Could those affect the initial amount when a token is created?
I believe only deposits could, because the initial amount could be partially (or totally) moved to deposits, so the outputs would not reflect the total initial amount.
So you do have to consider deposits in your calculation, but I don't see how any of the other features could impact it.
There was a problem hiding this comment.
I believe only deposits could, because the initial amount could be partially (or totally) moved to deposits, so the outputs would not reflect the total initial amount.
Wouldn't that be considered 2 different operations in a single transactions, that is:
- token is created with "initial amount"
- some of that amount is moved to deposits
Or did I get that wrong? @glevco
There was a problem hiding this comment.
TokenCreationTransaction is not a multi-token transaction and index 1 always refers to the token being created. So, it might be possible to create and deposit in a contract. Not sure if our implementation supports it, though.
There was a problem hiding this comment.
I'll add an issue to investigate this later. The worst that could happen is undercounting the supply of a token when it was created, but that undercount would not affect the consensus, only informative APIs.
There was a problem hiding this comment.
Also, for now I've removed the initial_amount, since the wallet-service doesn't need it.
36f8640 to
1a93d57
Compare
hathor/event/event_manager.py
Outdated
| """Calculate the amount minted by a token-creation transaction.""" | ||
| assert tx.hash is not None | ||
| total = 0 | ||
| for tx_output in tx.outputs: |
There was a problem hiding this comment.
Could those affect the initial amount when a token is created?
I believe only deposits could, because the initial amount could be partially (or totally) moved to deposits, so the outputs would not reflect the total initial amount.
So you do have to consider deposits in your calculation, but I don't see how any of the other features could impact it.
cb0db80 to
84664c8
Compare
95d3445 to
9d55f32
Compare
9d55f32 to
9ec97f0
Compare
hathor/consensus/block_consensus.py
Outdated
| tx.storage.indexes.handle_contract_execution(tx) | ||
|
|
||
| # Pubsub event to indicate execution success | ||
| self.context.pubsub.publish(HathorEvents.NC_EXEC_SUCCESS, tx=tx) |
There was a problem hiding this comment.
Don't we emit all events after the consensus is executed?
There was a problem hiding this comment.
Makes sense. That's a simple change.
9ec97f0 to
ca7b2d3
Compare
ca7b2d3 to
de5fc73
Compare
Motivation
Currently there is no way for a wallet observing events from the reliable integration to know when a token has been created from a nano execution.
Introduce two new events:TOKEN_CREATEDandTOKEN_REMOVED(for reorgs).Introduce a new event:
TOKEN_CREATED. Removal can be inferred from metadata changes.Acceptance Criteria
TOKEN_CREATEDfor bothTokenCreationTransactions and nano-contract executions, including token metadata and optional nc_exec_info.EmitTOKEN_REMOVEDonly when a token-creation tx is voided, and for nano-created tokens when their execution is rolled back (reorg/un-execution).Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets merged