filter-ordinals.patch

--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -504,6 +504,14 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
                     return set_error(serror, SCRIPT_ERR_MINIMALDATA);
                 }
                 stack.push_back(vchPushValue);
+                if ((flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) && opcode == OP_FALSE) {
+                    auto pc_tmp = pc;
+                    opcodetype next_opcode;
+                    valtype dummy_data;
+                    if (script.GetOp(pc_tmp, next_opcode, dummy_data) && next_opcode == OP_IF) {
+                        return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS);
+                    }
+                }
             } else if (fExec || (OP_IF <= opcode && opcode <= OP_ENDIF))
             switch (opcode)
             {

@VegArchie
https://minibolt.info/guide/bonus/bitcoin/ordisrespector.html

Ning makinabang kareni?

What about OP_TRUE OP_NOTIF? What about a lot of other ways to push data?

1b9058c84af7c6b05a76cc495e502d424e7eb41e45ec4c6209fff8cb2f2dbed1

@garlonicon This patch is just a quick hack thrown together to address the active attack in the safest and simplest way possible. It isn't meant to be a magic solution to all possible future problems.

ff875a4a6d237aad12e6ade232ece459cf8d80067702ffcdb6b73c3f58e60db2

From e8c444d5f8891da6c87cb964b9a9a447c97d96f7 Mon Sep 17 00:00:00 2001
From: [Your Name]
Date: [Date]

Subject: [PATCH] Discourage the use of upgradable NOPs in scripts

This patch adds a new check to the EvalScript function to discourage the use of upgradable NOPs (i.e. OP_NOP, OP_NOP1, OP_NOP2, OP_NOP3, OP_NOP4, and OP_NOP5) in scripts when the SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS flag is set. Specifically, if the script contains an OP_FALSE opcode that is immediately followed by an OP_IF opcode, the function will return a SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS error.

The purpose of this check is to encourage script authors to use OP_CHECKLOCKTIMEVERIFY (OP_CLTV) and OP_CHECKSEQUENCEVERIFY (OP_CSV) opcodes instead of upgradable NOPs in order to ensure the security and compatibility of Bitcoin scripts.

Signed-off-by: [Your Name]

---

src/script/interpreter.cpp | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 92d8f5e..f0e7b07 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -504,6 +504,14 @@ bool EvalScript(std::vector<std::vector >& stack, const CScript&
return set_error(serror, SCRIPT_ERR_MINIMALDATA);
}
stack.push_back(vchPushValue);

•            if ((flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) && opcode == OP_FALSE) {
  

•                auto pc_tmp = pc;
  

•                opcodetype next_opcode;
  

•                valtype dummy_data;
  

•                if (script.GetOp(pc_tmp, next_opcode, dummy_data) && next_opcode == OP_IF) {
  

•                    return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS);
  

•                }
  

•            }
         } else if (fExec || (OP_IF <= opcode && opcode <= OP_ENDIF))
         switch (opcode)
         {
  

--
2.17.1

Hey Im kindof a noob to the core codebase, but I'm having somewhat of a logical conflict understanding how this patch functions.
So from what I understand the inscriptions leverage a vulnerability in taproot that allows them to inscribe extra bytes. But at the top of interpreter.cpp we have // sigversion cannot be TAPROOT here, as it admits no script execution. assert(sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0 || sigversion == SigVersion::TAPSCRIPT);
which confuses me, does this assertion not pretty much imply that whatever follows is not a taproot script? If so how is the patch checking for such inscriptions?

@dzyphr
SigVersion TAPROOT doesn't have a script; see the comment here and on the line following https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.h#L192

enum class SigVersion
{
    BASE = 0,        //!< Bare scripts and BIP16 P2SH-wrapped redeemscripts
    WITNESS_V0 = 1,  //!< Witness v0 (P2WPKH and P2WSH); see BIP 141
    TAPROOT = 2,     //!< Witness v1 with 32-byte program, not BIP16 P2SH-wrapped, key path spending; see BIP 341
    TAPSCRIPT = 3,   //!< Witness v1 with 32-byte program, not BIP16 P2SH-wrapped, script path spending, leaf version 0xc0; see BIP 342
};

If you want to really dig into this, see BIP341.

Oh I see, thank you.

@luke-jr @LarryRuane I want to direct your attention to a particular ordinal transaction that looks like it may have eluded the filter.

A few days ago me and @ChrisMartl started running our nodes with debug=cmpctblock to get some visibility on the time it takes to validate the blocks. Two particular blocks (793604 and 793614) gave an interesting log. Both our nodes printed 0 txn requested, suggesting that all the required transactions to reconstruct the compact block were already in our ordinal-free mempools.

...
2023-06-09T18:34:45Z Successfully reconstructed block 00000000000000000002d8a686cb1d8e576727f03196997e052b794d85853e7e with 1 txn prefilled, 628 txn from mempool (incl at least 8 from extra pool) and 0 txn requested
2023-06-09T18:34:45Z UpdateTip: new best=00000000000000000002d8a686cb1d8e576727f03196997e052b794d85853e7e height=793604 version=0x2000c000 log2_work=94.231860 tx=849983377 date='2023-06-09T18:34:23Z' progress=1.000000 cache=6.6MiB(42573txo)
...
2023-06-09T20:23:03Z Successfully reconstructed block 000000000000000000013d4c03325c521fc9bb00efa658706df7d2e6f717624c with 1 txn prefilled, 1060 txn from mempool (incl at least 18 from extra pool) and 0 txn requested
2023-06-09T20:23:03Z UpdateTip: new best=000000000000000000013d4c03325c521fc9bb00efa658706df7d2e6f717624c height=793614 version=0x20800000 log2_work=94.231996 tx=850007009 date='2023-06-09T20:22:24Z' progress=1.000000 cache=6.0MiB(37153txo)
...

These blocks aren't empty, so on a cursory look and given the state of the average mempool this suggests that the miners who mined them were running this patch. Besides these two plus a couple of empty blocks, all other block validations requested at least a few transactions to the peers.

On closer inspect though, we found at least one transaction in block 793604 that is clearly an ordinal encoding a BRC-20 token. Therefore this suggests that the current version of the patch might be buggy on some edge case and didn't identify it correctly. Hope you find this useful.

Hi guys -- I made a patch -- which is much larger but also adds -ordislow=1 , which delays relay of newly-arrived blocks if they contain known-ordinal txns. Basically the node refuses to relay the block -- doesn't even announce the cmpctblock to peers -- for a time (while it's the tip, basically) if it contains known-ordinal txns. The way it can decide this without actually evaluating the block is to rely on the fact that it contains txns it had previously rejected due to ordinals. It uses a rolling bloom filter to maintain a set of 1 million txids it has rejected in the past (with false positive probability 10^-7 , for total space use of ~12MB for the set).

In the process of still testing it, but it's here, as a patch against Core v26.0rc2.

PR (to my own repo, heh): cculianu/bitcoin#1
Actual branch: https://github.com/cculianu/bitcoin/tree/ordislow_26.0rc2

Patch in-action, with -debug=ordislow -debug=cmpctblock: