Skip to content

Zk bags events#61

Open
EasonC13 wants to merge 3 commits intoMystenLabs:mainfrom
EasonC13:zk_bags_events
Open

Zk bags events#61
EasonC13 wants to merge 3 commits intoMystenLabs:mainfrom
EasonC13:zk_bags_events

Conversation

@EasonC13
Copy link

No description provided.


/// Event emitted when a ZkBag is destroyed after all items are claimed
public struct BagDestroyedEvent has copy, drop {
bag_id: ID,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The contract requires that all items are claimed, and the bag is destroyed in the same transaction, so I'm not sure this makes sense as a separate event

public struct BagItemAddedEvent<phantom T> has copy, drop {
bag_id: ID,
creator: address,
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this event is useful without the id of the added item. In practice items won't be added to links outside the creating transaction, so this is effectively just a duplicate of the create event that let's you count the number of items in the link

public struct BagItemClaimedEvent<phantom T> has copy, drop {
bag_id: ID,
creator: address,
receiver: address,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comment as the add item event, expect here we enforce that all items will be claimed together

@EasonC13
Copy link
Author

EasonC13 commented Nov 28, 2024

I'm not sure if this event is useful without the id of the added item.

Yes I think that too, but we don't know coin value even if we know ID. Actually I recommended we separate Coin and Object (NFT) deposit like I implemented at Buck You, which is Bucket version's zkSend https://github.com/Bucket-Protocol/Buck-You-Move/blob/main/sources/locker.move

"The contract requires that all items are claimed... so I'm not sure this makes sense as a separate event"
"expect here we enforce that all items will be claimed together"

but what if someone do it differently? Then we can catch it by checking event.

@hayes
Copy link

hayes commented Nov 28, 2024

but what if someone do it differently? Then we can catch it by checking event.

The contract enforced this, so it can't be done differently

@EasonC13
Copy link
Author

OK, thank you for the clarification, here is the updated PR with commit 130373e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants