Skip to content

add WearerStandingChanged event#81

Merged
spengrah merged 3 commits intodevelopfrom
feat/eligibility-event
Jan 11, 2023
Merged

add WearerStandingChanged event#81
spengrah merged 3 commits intodevelopfrom
feat/eligibility-event

Conversation

@spengrah
Copy link
Copy Markdown
Member

@spengrah spengrah commented Jan 6, 2023

This PR adds a WearerStandingChanged event emitted any time a wearer's standing for a given hat is changed.

It also adds a check within _processHatWearerStatus to ensure that burning a non-existent hat is not attempted. This is important because its possible for both eligible and standing values to change for a given wearer even if they are not actually wearing the hat in question.

@spengrah spengrah requested a review from nintynick January 6, 2023 19:04
@spengrah spengrah marked this pull request as ready for review January 6, 2023 19:22
@spengrah spengrah changed the base branch from feat/16x14 to develop January 8, 2023 00:09
Copy link
Copy Markdown
Member

@nintynick nintynick left a comment

Choose a reason for hiding this comment

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

LGTM. some highlights from our discussion of this:

Why not emit eligible as part of the event?

the source of truth for eligibility is not transaction records, its the eligibility contract itself. so data indexed from events is not going to be the source of truth, and apps are going to have to read directly from the contract to check whether a wearer is eligible. but since we explicitly store badStandings, and storage can only change as a result of a tx, data indexed based on an event can be the source of truth

we take the other approach for wearing, which is based on both eligibility and hat status. But that’s because the 1155 standard forces us to emit events when hats are minted/transfered. Apps will need to confirm hat wearer status by reading from the contract, even if the subgraph tells them an address is wearing a hat.

if they don't, they could show out-of-date information about whether an account is eligible for or wearing a given hat. token gates will always explicitly check the contract, so no issues there. but more informational / visualization front ends will need to use a mix of subgraph and explicit on-chain reads

dev documentation will definitely be important here. When we build an SDK we can have the SDK handle that more seamlessly, too

@spengrah spengrah merged commit 14776ed into develop Jan 11, 2023
@spengrah spengrah deleted the feat/eligibility-event branch January 11, 2023 01:04
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