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.

Sign up for GitHub

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

Test validity of all opcodes #3461

Merged
merged 2 commits into from
Apr 11, 2024
Merged

Test validity of all opcodes #3461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c64d8d
Test validity of all opcodes
casey Apr 3, 2024
397b811
Fix lint
casey Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions crates/ordinals/src/runestone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2104,4 +2104,83 @@ mod tests {
}),
);
}

#[test]
fn all_pushdata_opcodes_are_valid() {
for i in 0..79 {
let mut script_pubkey = Vec::new();

script_pubkey.push(opcodes::all::OP_RETURN.to_u8());
script_pubkey.push(Runestone::MAGIC_NUMBER.to_u8());
script_pubkey.push(i);

match i {
0..=75 => {
for j in 0..i {
script_pubkey.push(if j % 2 == 0 { 1 } else { 0 });
}

if i % 2 == 1 {
script_pubkey.push(1);
script_pubkey.push(1);
}
Comment on lines +2119 to +2126
Copy link

@summraznboi summraznboi Apr 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is how I implemented this unit test in Typescript:

for (const j of _.range(0, i)) {
  scriptPubKeyBuffers.push(Buffer.from([j % 2]));
}

if (i >= 2) {
  const additionalIntegersCount = (77 - i) % 4;
  const additionalIntegers = _.reverse(
    _.range(0, additionalIntegersCount).map((i) => i % 2)
  );
  scriptPubKeyBuffers.push(Buffer.from([additionalIntegersCount, ...additionalIntegers]));
}

This basically adds the necessary integers to avoid the cenotaph flaw of trailing integers.

}
76 => {
script_pubkey.push(0);
}
77 => {
script_pubkey.push(0);
script_pubkey.push(0);
}
78 => {
script_pubkey.push(0);
script_pubkey.push(0);
script_pubkey.push(0);
script_pubkey.push(0);
}
_ => unreachable!(),
}

assert_eq!(
Runestone::decipher(&Transaction {
version: 2,
lock_time: LockTime::ZERO,
input: default(),
output: vec![TxOut {
script_pubkey: script_pubkey.into(),
value: 0,
},],
})
.unwrap(),
Artifact::Runestone(Runestone::default()),
);
}
}

#[test]
fn all_non_pushdata_opcodes_are_invalid() {
Copy link

Choose a reason for hiding this comment

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

For demonstration purposes, I would specifically test OP_PUSHNUM in contexts that would otherwise be correct if replaced with its equivalent OP_PUSHBYTES, ideally a single edict whose amount is controlled by the OP_PUSHNUM opcode.

Copy link

Choose a reason for hiding this comment

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

Example:

test('all_pushnum_opcodes_are_invalid', () => {
  for (let i = 1; i <= 16; i++) {
    {
      const opcode = opcodes.OP_RESERVED + i;
      const scriptPubKey = Buffer.from([OP_RETURN, MAGIC_NUMBER, 3, 0, 1, 0, opcode, 1, 0]);
      expect(
        isRunestone(
          Runestone.decipher({
            vout: [{ scriptPubKey: { hex: scriptPubKey.toString('hex') } }],
          }).unwrap()
        )
      ).toBe(false);
    }

    {
      const scriptPubKey = Buffer.from([OP_RETURN, MAGIC_NUMBER, 3, 0, 1, 0, 1, i, 1, 0]);
      expect(
        isRunestone(
          Runestone.decipher({
            vout: [{ scriptPubKey: { hex: scriptPubKey.toString('hex') } }],
          }).unwrap()
        )
      ).toBe(true);
    }
  }
});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm hesitant to add such a test, just because I know that the code doesn't care where the opcode is in the runestone.

for i in 79..=u8::MAX {
assert_eq!(
Runestone::decipher(&Transaction {
version: 2,
lock_time: LockTime::ZERO,
input: default(),
output: vec![TxOut {
script_pubkey: vec![
opcodes::all::OP_RETURN.to_u8(),
Runestone::MAGIC_NUMBER.to_u8(),
i
]
.into(),
value: 0,
},],
})
.unwrap(),
Artifact::Cenotaph(Cenotaph {
flaws: Flaw::Opcode.into(),
..default()
}),
);
}
}
}
9 changes: 6 additions & 3 deletions docs/src/runes/specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,12 @@ OP_13`. If deciphering fails, later matching outputs are not considered.

#### Assembling the Payload Buffer

The payload buffer is assembled by concatenating data pushes. If a non-data
push opcode is encountered, the deciphered runestone is a cenotaph with no
etching, mint, or edicts.
The payload buffer is assembled by concatenating data pushes, after `OP_13`, in
the matching script pubkey.

Data pushes are opcodes 0 through 78 inclusive. If a non-data push opcode is
encountered, i.e., any opcode equal to or greater than opcode 79, the
deciphered runestone is a cenotaph with no etching, mint, or edicts.

#### Decoding the Integer Sequence

Expand Down
Loading