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

Polls RFC 69 #320

Open
wants to merge 61 commits into
base: master
Choose a base branch
from
Open

Conversation

toadlyBroodle
Copy link

@toadlyBroodle toadlyBroodle commented Mar 3, 2023

Request for Comment on new Poll event (kind #6969)
-Poll notes are paid poll events on nostr used to conduct quantitative opinion polls that users can vote on with zaps

-add poll format request for comment (RFC) pro/con lists
-remove placeholding json structure
-further define poll event
-further define purpose
-define outcomes section
-outline format
-add detailed 'count' tally method requirements
-add option tag reference
-remove bolt11 references
-add standardized poll event json format
-reorder, refine poll format
-modify TODOs
-add zap vote format section
-edit, elaborate other sections
-organize and clarify format sections
-other minor edits
-clarify, organize outcome and other sections
-minor edits
-add poll tags to standardized tags index
Copy link
Collaborator

@Semisol Semisol left a comment

Choose a reason for hiding this comment

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

this looks great, but needs some changes. also, in the future, we may need polls that count people that fall under specific criteria, having a NIP-05, 2 degrees of separation from the user that does the poll, ... but that can be left for later

01.md Outdated Show resolved Hide resolved
69.md Outdated Show resolved Hide resolved
69.md Outdated Show resolved Hide resolved
69.md Outdated Show resolved Hide resolved
69.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
toadlyBroodle and others added 2 commits March 5, 2023 11:02
… consensus_threshold and closed_at tags to type string, remove poll tags from README tags index
Copy link
Author

@toadlyBroodle toadlyBroodle left a comment

Choose a reason for hiding this comment

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

Reverted unnecessarily changed files with CRLFs, added poll_option references to 57.md examples

@fiatjaf fiatjaf changed the title Polls RFC #69 Polls RFC 69 May 5, 2023
@vitorpamplona
Copy link
Collaborator

Can we merge this? Amethyst and Snort implement this NIP and has been working well.

@fiatjaf
Copy link
Member

fiatjaf commented Jul 6, 2023

I haven't merged it yet because it is breaking the README and I haven't had time for or remembered fixing it myself.

@staab
Copy link
Member

staab commented Jul 6, 2023

I realize this ship has already sailed, but NACK on making zaps a hard dependency of other nostr features. Payment proof should be open ended and optional.

@alexgleason
Copy link
Member

Agreed @staab. We will need a different type of poll to bridge polls from ActivityPub.

@vitorpamplona
Copy link
Collaborator

@staab @alexgleason I don't see this as the only poll implementation in Nostr and I generally agree that the dependency on zaps is not good. However, I do think for those using zaps, this is already good enough.

@fernandolguevara
Copy link
Contributor

@staab @alexgleason I don't see this as the only poll implementation in Nostr and I generally agree that the dependency on zaps is not good. However, I do think for those using zaps, this is already good enough.

I think we can make this better, Dependency on zaps sucks...
We need a better way to make this use case over the network ...
#111

@toadlyBroodle
Copy link
Author

I haven't merged it yet because it is breaking the README and I haven't had time for or remembered fixing it myself.

Fixed

@vitorpamplona
Copy link
Collaborator

I am coming here again asking to merge this. Let's move on.

@staab
Copy link
Member

staab commented Aug 8, 2023

Consensus seems to be that dependency on zaps is bad. I vote against this PR.

@vitorpamplona
Copy link
Collaborator

vitorpamplona commented Aug 8, 2023

Consensus seems to be that dependency on zaps is bad. I vote against this PR.

Can't we merge and if somebody has a good non-zap implementation they can just update this NIP? By not merging, we are essentially blocking all potential changes from the community.

@staab
Copy link
Member

staab commented Aug 8, 2023

There have been 3-4 other poll implementations. We could vote on those or someone could make a new NIP. I'd rather not merge for no reason and make a bad idea a standard. Do we have a tally of how many implementations each one has? We've had polls in various places for over a year, I remember some in branle.

@vitorpamplona
Copy link
Collaborator

make a bad idea a standard

That's a bit too much. Not having a non-zap version doesn't make this a bad idea. The procedure is sound as a polling option. If people want to offer polls with zaps, this is it. We can tweak things around to make it better, but it works well.

Then somebody else will need to create a different procedure for polling without zaps that hopefully have similar protections against spam.

@toadlyBroodle
Copy link
Author

toadlyBroodle commented Aug 11, 2023

There have been 3-4 other poll implementations.

I've yet to see a single other NIP that comes anywhere close to as comprehensive as this one. Afterall, NIP69 was paid out @NVK and @StackSatsIO bounty.

Copy link
Collaborator

@Semisol Semisol left a comment

Choose a reason for hiding this comment

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

I'll review this more later.

@@ -37,6 +37,7 @@ In addition, the event MAY include the following tags:

- `e` is an optional hex-encoded event id. Clients MUST include this if zapping an event rather than a person.
- `a` is an optional NIP-33 event coordinate that allows tipping parameterized replaceable events such as NIP-23 long-form notes.
- `poll_option` is a tag used for voting by [zap poll events](69.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

No reason to modify NIP-57 for this.

Copy link
Author

Choose a reason for hiding this comment

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

This is under optional MAY conditional list, for documentation purposes. It is a required field for zap-poll event zap requests: which is why it's included here. Still want it removed?

README.md Show resolved Hide resolved
69.md Outdated Show resolved Hide resolved
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.