Skip to content

Experimental Inscription Support#3

Open
JeremyRubin wants to merge 6 commits into
masterfrom
inscribor
Open

Experimental Inscription Support#3
JeremyRubin wants to merge 6 commits into
masterfrom
inscribor

Conversation

@JeremyRubin

Copy link
Copy Markdown

No description provided.

Comment thread src/miniscript/lex.rs Outdated
if ret.last() == Some(&Token::Num(0)) {
// Inscription Detected
ret.pop();
if let Some(Ok(Instruction::PushBytes(b"ord"))) = it.next() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

might be good to use PROTOCOL_ID instead of b"ord" here since you already have it as a const, and in-case you ever want to parse other envelope-using protocols

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the reason is more annoying than that. it's because pattern matching on consts is IMO sketchy... to make it work "right" you want to do ord::PROTOCOL_ID (so it isn't accidentally an unchecked ID), and even then, I couldn't get the compiler to accept it because array/slice stuff. If you know how to get it to work LMK.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yeah, you're right :-/ unless you wanted to do something like

if let Some(Ok(Instruction::PushBytes(bytes))) = it.next() {
    if bytes == PROTOCOL_ID {
        // whatever
    }
}

this approach is better

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

the compiler is no match for me.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I kinda want to stick a comment in there not to remove the name ord::

Comment thread src/ord/envelope.rs
let metaprotocol = Tag::Metaprotocol.remove_field(&mut fields);
let parent = Tag::Parent.remove_field(&mut fields);
let pointer = Tag::Pointer.remove_field(&mut fields);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

there's also an unbound tag (66). are you just having it get rolled into unrecognized_even_field?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I coppied this code from the ord repo so i'd do whatever they say! is 66 good?

Comment thread src/lib.rs
pub mod policy;
pub mod psbt;
#[allow(missing_docs)]
pub mod ord;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would it be useful to you if we exported the inscriptions module in ord and made more types public so you can use it as a crate?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yes, I think it's a good idea. but also not really for sapio unfortunately since we have like the entire rust-bitcoin ecosystem forked with patches.

this patch set though should be upstreamable!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Would it be useful to you if we exported the inscriptions module in ord and made more types public so you can use it as a crate?

I would love that.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Have a look if this is useful: ordinals/ord#3042

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