[BUG] AccountUpdated value ambiguity
Closed this issue · 1 comments
The AccountUpdated event has the same values for removal from a Manager as being published for a parent account.
// Manager.removeChild() emits
AccountUpdated(id: id, child: cap.address, parent: self.owner!.address, active: false)
// OwnedAccount.publishToParent() emits
AccountUpdated(id: self.uuid, child: self.owner!.address, parent: parentAddress, active: false)This ambiguity can make it difficult to deduce state changes. These two actions should emit distinct event values.
Closing comms loop on conversations about this bug - since this is the second time issues with events have has emerged, the thought is we make contract events more granular. Event consolidation was originally suggested due to the difficulty around filtering large number of contract related events; however, recent changes to the event streaming API largely solve this issue.
As such, I think we should deconsolidate the OwnershipUpdated event into two, similar in pattern to AccountUpdated & ChildAccountPublished. That is, we maintain OwnershipUpdated and add OwnershipGranted, the latter of which is emitted in OwnedAccount.giveOwnership(). We additionally remove the AccountUpdated emitted in OwnedAccount.publishToParent().
With these changes, we get coverage of the major updates (add/remove) & publish actions a user and/or publisher would be concerned with:
child: 0x01 | parent: 0x02
| action | event |
|---|---|
Manager.addAccount() |
AccountUpdated(id: XX, child: 0x01, parent: 0x02, active: true) |
Manager.removeAccount() |
AccountUpdated(id: XX, child: Address, parent: Address, active: Bool) |
Manager.addOwned() |
OwnershipUpdated(id: XX, child: 0x01, previousOwner: 0xXX, owner: 0x02, active: true) |
Manager.removeOwned() |
OwnershipUpdated(id: XX, child: 0x01, previousOwner: 0x02, owner: nil, active: false) |
OwnedAccount.publishToParent() |
ChildAccountPublished(..., child: 0x01, pendingParent: 0x02) |
OwnedAccount.giveOwnership() |
OwnershipGranted(..., child: 0x01, previousOwner: 0xXX, pendingOwner: 0x02) |
Here's what I think complete event signature would look like:
OwnershipGranted(ownedAcctID: UInt64, child: Address, previousOwner: Address?, pendingOwner: Address)
Effectively, XUpdated events can be taken to mean ChildAccount or OwnedAccount Capabilities were added/removed to/from Manager while ChildAccountPublished and OwnershipGranted mean ChildAccount and OwnedAccount have new parent/owner respectively.
I'll submit a PR with these suggested changes as a point of conversation here in a bit.