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

Allow inscribing a delegate without content #3027

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

Conversation

lifofifoX
Copy link
Collaborator

This will allow using wallet inscribe without any batch or a file, resulting in smaller tx size.

Todo:

  • Add tests
  • Fail if all 3 options (file, batch and delegate) are missing

@raphjaph raphjaph changed the title Allow inscribing a delegate without any batch or a file Allow inscribing a delegate without content Jan 23, 2024
* master:
  Dump and restore wallet from descriptors (ordinals#3048)
  Forbid destinations in same-sat mode (ordinals#3038)
  Enable JSON API by default (ordinals#3047)
  Exclude unnecessary docs (ordinals#3043)
  Add documentation for reinscriptions (ordinals#2963)
  Better wallet error messages (ordinals#3041)
  Remove uneccessary allocations in Inscription Script Creation (ordinals#3039)
  Test fee-spent inscription numbering (ordinals#3032)
  Break deploy recipes into multiple lines (ordinals#3026)
  Make wallet communicate with index via RPC (ordinals#2929)
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

The overall logic of delegate should be that you can inscribe a delegate only with

ord wallet inscribe --delegate <INSCRIPTION_ID>

You may inscribe a delegate that doesn't exist with:

ord wallet inscribe --delegate <INSCRIPTION_ID> --allow-non-existant

And you can also inscribe a file in addition to both of these options as a fall back.

@@ -108,6 +108,26 @@ impl Inscription {
})
}

pub(crate) fn without_file(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pub(crate) fn without_file(
pub(crate) fn delegate_only(

match (self.file, self.batch) {
(Some(file), None) => {
match (self.file, self.batch, self.delegate) {
(Some(file), None, _) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This would mean if we have a file and a delegate it would still inscribe the file. Same with the batch below. This should be tested.

Comment on lines +156 to +159
ensure! {
wallet.inscription_exists(delegate)?,
"delegate {delegate} does not exist"
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's add a flag called --allow-non-existant-delegate and then skip this check if it is set.

Copy link

Choose a reason for hiding this comment

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

what is the expected response for non-existent-delegate w/o payload and existent-delegate w/ payload?

@raphjaph
Copy link
Collaborator

What's the status on this?

@ep150de
Copy link

ep150de commented Oct 7, 2024

Had worked around this when inscribing using batch delegate, I use a 1 byte dummy file so I can minimize the extra unnecessary cost and block space, hope this PR gets progress so I don't have to do that going forward, some markets make it think the ordinal is literally a 1 byte blank file when it should show the delegate content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for Review
Development

Successfully merging this pull request may close these issues.

Inscribing with delegate shouldn't inscribe file contents
4 participants