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

Delegation #2912

Merged
merged 11 commits into from
Dec 27, 2023
Merged

Delegation #2912

merged 11 commits into from
Dec 27, 2023

Conversation

casey
Copy link
Collaborator

@casey casey commented Dec 25, 2023

Allow inscriptions to nominate a delegate inscription, whose content is displayed instead of the inscription's.

@lifofifoX
Copy link
Collaborator

Could this potentially be used for generative art inscriptions that use inscription ID as seed? 🤔

@casey
Copy link
Collaborator Author

casey commented Dec 25, 2023

Yah, definitely. It would make them very efficient.

@lyndoco
Copy link

lyndoco commented Dec 25, 2023

As plain txt, as opposed to wrapping it in an html container?

@casey
Copy link
Collaborator Author

casey commented Dec 25, 2023

@lyndoco What do you mean?

@lyndoco
Copy link

lyndoco commented Dec 26, 2023

@casey are you saying that people are inscribing .txt files that simply contain "/content/asdfinsertidherepenisasdfi0"?

and you just wanna resolve that to rendering whatever the pointer is referencing?

Yeah, that would be dope - I'd be using this convention immediately.

You'd be saving me the scripting and html wrappers that I'm currently using...

and sum sweet sats

@casey
Copy link
Collaborator Author

casey commented Dec 26, 2023

@casey are you saying that people are inscribing .txt files that simply contain "/content/asdfinsertidherepenisasdfi0"?

Yes, they're creating inscriptions whose content are just the text /content/ID. They set the content type to the type of the inscription that they point to.

and you just wanna resolve that to rendering whatever the pointer is referencing?

I don't want to support it directly, since it's a pretty ugly hack. Instead this PR adds a new tag, which contains a binary inscription ID, which, if present, is displayed instead of the inscription contents. So same thing, but more efficient.

@lyndoco
Copy link

lyndoco commented Dec 26, 2023

@casey are you sure you don't wanna just parse all .txt files instead?

Yeah nah maybe we'll go with your idea...

@lifofifoX
Copy link
Collaborator

  1. It'd be cool to have a fallback when the delegation tag is present, but the value isn't. Maybe, the first parent?
  2. If the delegated inscription doesn't exist, perhaps it's better to attempt to serve the inscription's inscribed content instead of throwing an error. That could make it easy to do delayed reveals.

@casey
Copy link
Collaborator Author

casey commented Dec 26, 2023

  1. It'd be cool to have a fallback when the delegation tag is present, but the value isn't. Maybe, the first parent?

Can you elaborate? I'm not sure I understand what you mean.

  1. If the delegated inscription doesn't exist, perhaps it's better to attempt to serve the inscription's inscribed content instead of throwing an error. That could make it easy to do delayed reveals.

That seems reasonable.

@lifofifoX
Copy link
Collaborator

  1. It'd be cool to have a fallback when the delegation tag is present, but the value isn't. Maybe, the first parent?

Can you elaborate? I'm not sure I understand what you mean.

Say, the envelope looks like this:

OP_FALSE
OP_IF
  OP_PUSH "ord"
  OP_PUSH 11
OP_ENDIF

In that case, could attempt delegating to the first parent or any other sensible default. Delegating to parent would make creating children inscriptions even cheaper, where this makes sense. That said, I am not fully sure if the added obscurity is worth the blockspace savings.

@casey casey marked this pull request as ready for review December 27, 2023 20:56
@casey casey enabled auto-merge (squash) December 27, 2023 21:12
@casey casey merged commit 2db64cb into ordinals:master Dec 27, 2023
6 checks passed
@casey casey deleted the delegation branch December 27, 2023 21:18
@arik-so
Copy link
Contributor

arik-so commented Dec 29, 2023

FYI, nested delegation does not appear to be working properly yet, i. e. anointing a delegate that itself has a delegate.

@casey
Copy link
Collaborator Author

casey commented Dec 29, 2023

FYI, nested delegation does not appear to be working properly yet, i. e. anointing a delegate that itself has a delegate.

I'm not sure we should support this. If we do, the ord server could possibly wind up having to load a bunch of delegates to return the content of one inscription.

@casey
Copy link
Collaborator Author

casey commented Dec 29, 2023

Did you have a use-case in mind?

@arik-so
Copy link
Contributor

arik-so commented Dec 29, 2023

Unfortunately, the use case is (I think) quite probable user accidents. A user sees an inscription, likes the content, and decides to use it as a delegate, or even in the act of creating a collection with potentially multiple levels of parent/child nesting, misses the fact that the referenced inscription itself has a delegate, and possibly ends up wasting dozens of charmed satoshis.

@arik-so
Copy link
Contributor

arik-so commented Dec 29, 2023

I think a workaround could be resolving the full delegation path at indexing time, such that if such an inscription got used as a delegate later, instead of following the resolution path again, you just immediately jump to its destination.

@casey
Copy link
Collaborator Author

casey commented Dec 29, 2023

I think a workaround could be resolving the full delegation path at indexing time, such that if such an inscription got used as a delegate later, instead of following the resolution path again, you just immediately jump to its destination.

I think it's not possible to do this at index time, since an inscription may have a delegate that isn't inscribed yet. (Maybe people will want to do this for delayed reveals.) But we could, whenever looking up a delegate, if it's found, store the resolved delegate in the index, so that we never look up the same delegate twice.

@arik-so
Copy link
Contributor

arik-so commented Dec 29, 2023

True, delayed reveals definitely throw a wrench in index-time resolution, but I do hope you choose to go the path of opportunistic caching for nested delegation lookups. I am rather certain that this scenario will, alas, come up for many users.

@casey
Copy link
Collaborator Author

casey commented Dec 29, 2023

Created #2922 to track recursive delegation.

@arik-so
Copy link
Contributor

arik-so commented Dec 30, 2023

Sorry to keep piling on, but currently, when a delegate inscription doesn't exist yet, the shown content is simply blank. However, if the inscription does have its own content, I think it might possibly be prudent to show that inscribed content pending the delegate becoming available later.

The use case for that would be a prettier delayed reveal experience, so you could literally have an image of a loading icon or whatever (I'm not an artist, that's the best I can do).

If taken a little further, an inscription that has multiple delegate tags could keep advancing to the last available content. I. e. first delegate's content if it's available, second delegate's content otherwise, etc. If none of the delegates' content is available, show own content.

This might however be really weird behavior, or require absurd amounts of re-indexing, so feel free to discard this suggestion :)

EDIT: I would also note that currently, regardless of the delegate's availability, the shown attributes content type and content length reflect the inscription's own content, and not the delegate's.

hans-crypto added a commit to ordpool-space/ordpool-parser that referenced this pull request Jan 17, 2024
hans-crypto added a commit to ordpool-space/ordpool that referenced this pull request Jan 17, 2024
torkelrogstad pushed a commit to torkelrogstad/ord that referenced this pull request Mar 11, 2024
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.

5 participants