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

NUT-17: clarify ProofStates subscription example #213

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

elnosh
Copy link
Contributor

@elnosh elnosh commented Jan 28, 2025

This adds some clarification to the ProofStates subscription kind to specify that the expected WsNotification should be an array of proof states.

Copy link
Contributor

@ok300 ok300 left a comment

Choose a reason for hiding this comment

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

I think another section of this NUT might be affected by this change:

Important: If the subscription is accepted by the mint, the mint MUST first respond with the current state of the subscribed object and continue sending any further updates to it.

For example, if the wallet subscribes to a Proof.Y of a Proof that has not been spent yet, the mint will first respond with a ProofState with state == "UNSPENT". If the wallet then spends this Proof, the mint would send a ProofState with state == "PENDING" and then one with state == "SPENT". In total, the mint would send three notifications to the wallet.

So before, when subscribing to one object, the responses should be several notifications tracking the state changes of that object:

  • current state (for example UNSPENT)
  • every intermediary state (for example PENDING)
  • final state (SPENT)

This means, if subscribing to 2 objects (2 filters), there would be separate notifications for each object state change (6 notifications in total, one per object per state change).

With this PR's change, now that notifications can include state changes of multiple objects, the above quoted paragraphs become unclear. Should a notification for all tracked object states be sent when even 1 object state changes? For N objects and S state changes, that's N*S notifications. For the example above, that's still 6 notifications and therefore the new list in the notification brings no benefit.

Because of this, I tend to think it's better to stick to a single ProofState update per notification.

17.md Outdated Show resolved Hide resolved
@elnosh
Copy link
Contributor Author

elnosh commented Jan 30, 2025

I think another section of this NUT might be affected by this change:

hmm yeah I thought about that section and wasn't sure to leave it as is or change that too. I left it as is since it is talking about 1 proof but maybe it needs changing as well.

Should a notification for all tracked object states be sent when even 1 object state changes?

My intent was for it to only send a notification for objects that have changed. So if you are subscribed to 10 objects and only 5 change, then the notification will only include the changes for the 5 that changed.

For the example above, that's still 6 notifications and therefore the new list in the notification brings no benefit.

In that specific case where you are subscribed to multiple objects and only 1 changes, then yes, there's no benefit. However proofs are usually spent together so in most cases there will be multiple updates and in that case there is a benefit. If 5 proof states changed, now the mint would send one notification with the 5 updates rather than before where it would send 5 different notifications each containing the update for 1 proof.

Also why I think it needed clarification is because when referring to the notification payload for proof states the NUT points to use the response from NUT-07. The response in NUT-07 is an array of states.

{
  "states": [
    {
      "Y": <hex_str>,
      "state": <str_enum[STATE]>,
      "witness": <str|null>,
    },
    ...
  ]
}

@ok300
Copy link
Contributor

ok300 commented Jan 30, 2025

Good points, agree.

Then I'd suggest to change that section to what you described above, i.e. the notification should only contain updates for the ProofStates that changed.

@elnosh
Copy link
Contributor Author

elnosh commented Jan 30, 2025

cool, thanks for comments.

As suggested, added a note clarifying that the array should only contain proofs that changed. I also changed the example in the section you mentioned to use MintQuote but it conveys the same thing (that multiple updates should be sent).

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