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

NIP-11: Pay to relay clarification #1475

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

Conversation

kehiy
Copy link
Contributor

@kehiy kehiy commented Sep 3, 2024

For details check:

@staab
Copy link
Member

staab commented Sep 3, 2024

This PR does too many things, between changing formats and updating actual NIP semantics. Can you split them up? BTW, I don't think expanding the json to one line per character is helpful for readability.

@kehiy kehiy changed the title NIP-11: pay to relay update and enhancement NIP-11: Pay to relay clarification Sep 3, 2024
@kehiy
Copy link
Contributor Author

kehiy commented Sep 3, 2024

@staab Style changed removed, I'll move them to another PR. Also, the reason for expanding this JSON (pay-to-relay document example) is to add comments and make it clearer to understand.

@staab
Copy link
Member

staab commented Sep 3, 2024

This looks good to me, but we'll want someone who has implemented this to confirm that it's all accurate. Maybe nostr.wine?

@kehiy
Copy link
Contributor Author

kehiy commented Sep 3, 2024

Here is wines document:

{
  "contact": "[email protected]",
  "description": "A paid nostr relay for wine enthusiasts and everyone else.",
  "fees": {
    "admission": [
      {
        "amount": 18888000,
        "unit": "msats"
      }
    ]
  },
  "icon": "https://i.nostr.build/567z.jpg",
  "limitation": {
    "auth_required": false,
    "created_at_lower_limit": 94608000,
    "created_at_upper_limit": 300,
    "max_event_tags": 4000,
    "max_limit": 1000,
    "max_message_length": 524288,
    "max_subid_length": 71,
    "max_subscriptions": 50,
    "min_pow_difficulty": 0,
    "payment_required": true,
    "restricted_writes": true
  },
  "name": "nostr.wine",
  "payments_url": "https://nostr.wine/invoices",
  "pubkey": "4918eb332a41b71ba9a74b1dc64276cfff592e55107b93baae38af3520e55975",
  "software": "https://nostr.wine",
  "supported_nips": [
    1,
    2,
    4,
    9,
    11,
    40,
    42,
    50,
    94
  ],
  "version": "0.3.2"
}

They are only providing admission rule, but based on their website I can see they have different models to do that.

But the admission is following these changes. or better to say it won't break any existing NIP-11 implementation.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 3, 2024

@staab @fiatjaf can you review the last changes?

@staab
Copy link
Member

staab commented Sep 5, 2024

Looks ok to me, but I don't trust myself to review this one correctly, it's an area of nostr I just haven't paid any attention to yet.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 5, 2024

@staab Cool, thanks. I think that's important to clarify this part. I think we can ask other friends to check this and make the final decision.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 8, 2024

Can someone help review this and make the final decision?

@vitorpamplona
Copy link
Collaborator

Can someone help review this and make the final decision?

Things are not merged until 2+ implementers have this change in production. That's your actual review. "Reviewers" here just try to see if things are aligned between NIPs. The rest is up to implementers. If this is being actually used, it will be merged.

As a client that consumes NIP-11, I don't think any of these new items are actionable to me. It's not that I can show these to the user and they are going to pay without any extra sales pitch to them.

@staab
Copy link
Member

staab commented Sep 8, 2024

What's new here? Seems like just clarification of what nip 11 already has.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 8, 2024

@staab Yes, there is nothing new. just adding the details of the existing NIP-11. which is required for someone to implement it properly. we need to clarify what are these fields and what they can contain as value.

@vitorpamplona FYI.

@vitorpamplona
Copy link
Collaborator

Let's make sure we have people actually using these fields. Otherwise, we should go in the opposite route and delete them entirely from the spec.

@kehiy
Copy link
Contributor Author

kehiy commented Sep 8, 2024

@vitorpamplona OK, I can see wine and nsotr land are using them at the moment. Also I have plan to use it on my own implementation. So, let't wait to see how it goes.

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