Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[framework] don't emit event on purchase with PurchaseCap #12821

Merged
merged 2 commits into from
Jul 5, 2023

Conversation

damirka
Copy link
Contributor

@damirka damirka commented Jul 3, 2023

Motivation

PurchaseCap functionality is created specifically for extensions and logic which may or may not be considered trading. And emitting an event when "purchasing" with the Cap breaks indexers and makes little sense - ItemListed is not emitted, ItemDelisted as well.

Currently, all extensions rely on custom events for this purpose.

Description

Changes the body of the kiosk::purchase_with_cap to NOT emit the ItemPurchased event. Everything else is left intact.

Test Plan

Current tests pass.

Type of Change (Check all that apply)

  • protocol change
  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

  • The kiosk::purchase_with_cap method for Sui Kiosk no longer emits the ItemPurchased event. This is a network optimization update that should have minimal impact. If the logic of your application does rely on this event, you must change it to leverage custom events instead.

@damirka damirka requested review from amnn and manolisliolios July 3, 2023 11:49
@damirka damirka self-assigned this Jul 3, 2023
@vercel
Copy link

vercel bot commented Jul 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

6 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Jul 5, 2023 5:28pm
explorer-storybook ⬜️ Ignored (Inspect) Jul 5, 2023 5:28pm
multisig-toolkit ⬜️ Ignored (Inspect) Jul 5, 2023 5:28pm
sui-kiosk ⬜️ Ignored (Inspect) Jul 5, 2023 5:28pm
sui-wallet-kit ⬜️ Ignored (Inspect) Jul 5, 2023 5:28pm
wallet-adapter ⬜️ Ignored (Inspect) Jul 5, 2023 5:28pm

@amnn
Copy link
Contributor

amnn commented Jul 4, 2023

@damirka, we're trying out an extra step in the release process for user facing changes, where we ping @randall-Mysten / @ronny-mysten on the docs channel to review the release notes before the change lands as well -- could you do that before landing please? Change looks good!

@ronny-mysten
Copy link
Contributor

I updated the Release notes section. Let me know if something doesn't make sense.

@damirka damirka force-pushed the ds-kiosk-dont-emit-event-on-pcap branch from 8504574 to 2bef4a8 Compare July 5, 2023 17:28
@damirka damirka merged commit 8e86b48 into main Jul 5, 2023
@damirka damirka deleted the ds-kiosk-dont-emit-event-on-pcap branch July 5, 2023 17:50
longbowlu pushed a commit that referenced this pull request Jul 6, 2023
## Motivation

PurchaseCap functionality is created specifically for extensions and
logic which may or may not be considered trading. And emitting an event
when "purchasing" with the Cap breaks indexers and makes little sense -
ItemListed is not emitted, ItemDelisted as well.

Currently, all extensions rely on custom events for this purpose.

## Description 

Changes the body of the `kiosk::purchase_with_cap` to NOT emit the
ItemPurchased event. Everything else is left intact.

## Test Plan 

Current tests pass.

### Type of Change (Check all that apply)

- [ ] protocol change
- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

- The `kiosk::purchase_with_cap` method for [Sui
Kiosk](https://docs.sui.io/build/sui-kiosk) no longer emits the
`ItemPurchased` event. This is a network optimization update that should
have minimal impact. If the logic of your application does rely on this
event, you must change it to leverage custom events instead.
ebmifa pushed a commit that referenced this pull request Jul 12, 2023
## Motivation

PurchaseCap functionality is created specifically for extensions and
logic which may or may not be considered trading. And emitting an event
when "purchasing" with the Cap breaks indexers and makes little sense -
ItemListed is not emitted, ItemDelisted as well.

Currently, all extensions rely on custom events for this purpose.

## Description 

Changes the body of the `kiosk::purchase_with_cap` to NOT emit the
ItemPurchased event. Everything else is left intact.

## Test Plan 

Current tests pass.

### Type of Change (Check all that apply)

- [ ] protocol change
- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes

- The `kiosk::purchase_with_cap` method for [Sui
Kiosk](https://docs.sui.io/build/sui-kiosk) no longer emits the
`ItemPurchased` event. This is a network optimization update that should
have minimal impact. If the logic of your application does rely on this
event, you must change it to leverage custom events instead.
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.

3 participants