multi: hook up burn and mint events to the supply commit state machine #1675
Conversation
address/book_test.go
Outdated
| // Set up mocks | ||
| tt.setupMocks(mockStorage) | ||
|
|
||
| // Call the method | ||
| hasDelegation, err := book.HasDelegationKey(ctx, tt.assetID) | ||
|
|
||
| // Check results | ||
| if tt.expectedError != "" { | ||
| require.Error(t, err) | ||
| require.Contains(t, err.Error(), tt.expectedError) | ||
| } else { | ||
| require.NoError(t, err) | ||
| } | ||
|
|
||
| require.Equal(t, tt.expectedHas, hasDelegation) | ||
|
|
||
| // Verify expectations |
There was a problem hiding this comment.
comments can be removed here
| // BurnSupplyCommitter is used to track supply changes (burns) and | ||
| // create periodic on-chain supply commitments. | ||
| BurnCommitter BurnSupplyCommitter | ||
|
|
||
| // DelegationKeyChecker is used to verify that we control the delegation | ||
| // key for a given asset, which is required for creating supply | ||
| // commitments. | ||
| DelegationKeyChecker address.DelegationKeyChecker |
There was a problem hiding this comment.
The ChainPorter already supports event emission, so I think we might be able to avoid leaking the concept of supply commitments into it, even if the logic is abstracted behind interfaces. Instead, the multi supply commit manager could subscribe as a consumer of ChainPorter events. Perhaps we define a new burn event emitted by the ChainPorter, and let the supply commit manager handle delegation key checks independently.
In other words, the ChainPorter just emits a burn event, and the multi supply commit manager does the rest. This would help keep the supply commitment feature cleanly separated.
There was a problem hiding this comment.
. Instead, the multi supply commit manager could subscribe as a consumer of ChainPorter events
What happens if the daemon crashes after that event is sent, but before the supply commit manager processes it? To make that durable, we'd need to have the event send in the main state machine loop as it's implemented now.
There was a problem hiding this comment.
So I generally agree with you, but we need that event emission to be durable.
One route that we can use to generally solve this problem is to extend the actor framework I added in lnd, to enable one to pass in a queue or mailbox like struct (rn it hard codes a normal Go channel). We'd then be able to make a new table of arbitrary events (with the requirement that they can be serialized), and have that back the durable mailbox for a given actor.
This would allow us to easily generalize this pattern between any sub-system we have. It would simplify some other sub-systems, as then they wouldn't need some reconciliation loop at start up, they'd just continue to reed from their queues.
There was a problem hiding this comment.
Related to this is that we have a similar gap right now with teh way the state machine works. The sends are non-blocking. What we want instead is that either the sens is blocking, or we wait on some extra channel/future until the update event has been written to disk.
To implement this, we can either implement a done call back in the event that we send over. Or extend the lnd state machine to make this a first class feature.
I lean towards the done call back, as we can do that in another PR and reduce update cycles.
There was a problem hiding this comment.
I agree, it feels better to do things more explicitly here, since the event system is more of a "fire-and-forget" kind of system.
…n control This commit introduces a new DelegationKeyChecker interface that allows verification of whether we control the delegation key for a given asset. The interface is implemented by the address.Book, which checks if the local key ring contains the private key corresponding to an asset's delegation public key. The implementation queries the asset group and metadata to extract the delegation key, then attempts to locate the corresponding private key in the local keystore. This verification is essential for ensuring only authorized parties can create supply commitment proofs for assets with delegation keys.
This commit extends the MultiStateMachineManager to implement the MintCommitter and BurnSupplyCommitter interfaces. These new methods provide a type-safe way to send mint and burn events to the supply commitment state machines. The SendMintEvent method accepts universe-specific types for minting operations, while SendBurnEvent handles burn leaf submissions. Both methods internally construct the appropriate event types and delegate to the existing SendEvent infrastructure, maintaining consistency with the state machine architecture.
This commit enhances the GardenKit configuration by adding two new optional dependencies: DelegationKeyChecker and MintCommitter. These additions enable the garden subsystem to verify delegation key ownership and submit mint events to the supply commitment state machine. The GardenKit struct serves as the central configuration hub for the tapgarden package, and these new fields allow components like the BatchCaretaker to conditionally enable supply commitment functionality when the required dependencies are available.
This commit extends the ChainPorterConfig with DelegationKeyChecker and BurnSupplyCommitter dependencies to enable supply commitment tracking for asset burns. When processing burn events, the chain porter now filters assets to only submit supply commitment events for those where we control the delegation key. The sendBurnSupplyCommitEvents method uses functional filtering to pre-process the burn list, ensuring we only attempt to create supply commitments for assets we're authorized to manage. This prevents unnecessary processing and potential errors for assets where we lack delegation authority.
This commit enhances the BatchCaretaker to filter mint events based on delegation key ownership. The sendSupplyCommitEvents method now uses functional filtering to ensure supply commitment events are only sent for assets where we control the delegation key. Similar to the burn event filtering in tapfreighter, this change ensures the caretaker respects delegation authority when creating supply proofs for newly minted assets. The filtering happens early in the process to avoid unnecessary proof construction and event submission for assets we're not authorized to manage.
This commit completes the integration by wiring the address book's DelegationKeyChecker implementation to both the GardenKit and ChainPorterConfig. With this configuration, both minting and burning operations now properly verify delegation key ownership before submitting supply commitment events. The address book serves as the central authority for key ownership verification, leveraging its existing access to the key ring and asset metadata storage. This ensures consistent delegation checking across all supply commitment operations.
This commit introduces comprehensive test coverage for the delegation key filtering functionality in the BatchCaretaker. The tests verify that mint supply commitment events are only sent for assets where we control the delegation key. The test suite covers three key scenarios: all assets having delegation keys, partial delegation key ownership, and no delegation keys. Mock implementations of the MintCommitter and DelegationKeyChecker interfaces enable isolated testing of the filtering logic without requiring actual key management infrastructure.
This commit adds a unified test suite for burn supply commitment event processing in the ChainPorter. The tests verify delegation key filtering, error handling, and various burn scenarios through a table-driven approach with a helper function for cleaner test setup. The test coverage includes successful burn processing with mixed group keys, delegation key filtering scenarios, error handling for both the delegation checker and burn committer, handling of missing dependencies, and validation of invalid group key bytes. The setupChainPorterTest helper function eliminates repetitive mock configuration, making the tests more maintainable and focused on the scenarios being tested.
Roasbeef
force-pushed
the
state-machine-hookup
branch
from
58d8271 to
b1b0ad7
Compare
| } | ||
|
|
||
| log.Debugf("Sent supply commit burn event for asset %x", | ||
| assetID) |
There was a problem hiding this comment.
nit: this will double hex encode, can just use %s as we have a .String() method on the asset ID.
| // BurnSupplyCommitter is used to track supply changes (burns) and | ||
| // create periodic on-chain supply commitments. | ||
| BurnCommitter BurnSupplyCommitter | ||
|
|
||
| // DelegationKeyChecker is used to verify that we control the delegation | ||
| // key for a given asset, which is required for creating supply | ||
| // commitments. | ||
| DelegationKeyChecker address.DelegationKeyChecker |
There was a problem hiding this comment.
I agree, it feels better to do things more explicitly here, since the event system is more of a "fire-and-forget" kind of system.
| // Send supply commitment events for all minted assets before | ||
| // finalizing the batch. This ensures that supply commitments | ||
| // are tracked before the batch is considered complete. | ||
| err = b.sendSupplyCommitEvents( |
There was a problem hiding this comment.
Should we do this before we call MarkBatchConfirmed? If we shut down while sending the supply commit events, the batch will already be marked as confirmed in the DB, so it won't be re-tried on next startup (and things aren't done atomically here).
Pull Request Test Coverage Report for Build 16897047482Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
No description provided.