| --- 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) | |
| { |
I applied your patch to latest bitcoin core (no conflicts), and it did detect one transaction. But I don't know if it's an instance of the intended kind of transaction. I knew your code detected it because I set a breakpoint. Here's the stack backtrace (in case this is at all helpful):
#0 EvalScript (stack=std::vector of length 2, capacity 2 = {...}, script=..., flags=2097119, checker=..., sigversion=SigVersion::TAPSCRIPT, execdata=..., serror=0x7fff0ebd6f9c) at script/interpreter.cpp:488
#1 0x000055555611973e in ExecuteWitnessScript (stack_span=..., exec_script=..., flags=2097119, sigversion=SigVersion::TAPSCRIPT, checker=..., execdata=..., serror=0x7fff0ebd6f9c) at script/interpreter.cpp:1830
#2 0x000055555611a700 in VerifyWitnessProgram (witness=..., witversion=1, program=std::vector of length 32, capacity 32 = {...}, flags=2097119, checker=..., serror=0x7fff0ebd6f9c, is_p2sh=false) at script/interpreter.cpp:1944
#3 0x000055555611aae8 in VerifyScript (scriptSig=..., scriptPubKey=..., witness=0x7fff05467b30, flags=2097119, checker=..., serror=0x7fff0ebd6f9c) at script/interpreter.cpp:2001
#4 0x0000555555a90cd7 in CScriptCheck::operator() (this=0x7fff0ebd6f60) at ./src/validation.cpp:1645
#5 0x0000555555a914a3 in CheckInputScripts (tx=..., state=..., inputs=..., flags=2097119, cacheSigStore=true, cacheFullScriptStore=false, txdata=..., pvChecks=0x0) at ./src/validation.cpp:1740
#6 0x0000555555a8ab1f in (anonymous namespace)::MemPoolAccept::PolicyScriptChecks (this=0x7fff0ebd7760, args=..., ws=...) at ./src/validation.cpp:992
#7 0x0000555555a8c7d6 in (anonymous namespace)::MemPoolAccept::AcceptSingleTransaction (this=0x7fff0ebd7760, ptx=std::shared_ptr<const CTransaction> (use count 2, weak count 0) = {...}, args=...) at ./src/validation.cpp:1175
#8 0x0000555555a8ed9d in AcceptToMemoryPool (active_chainstate=..., tx=std::shared_ptr<const CTransaction> (use count 2, weak count 0) = {...}, accept_time=1675367849, bypass_limits=false, test_accept=false) at ./src/validation.cpp:1417
#9 0x0000555555aa24f0 in ChainstateManager::ProcessTransaction (this=0x5555568aad40, tx=std::shared_ptr<const CTransaction> (use count 2, weak count 0) = {...}, test_accept=false) at ./src/validation.cpp:3933
#10 0x0000555555694c6c in (anonymous namespace)::PeerManagerImpl::ProcessMessage (this=0x555563f49ea0, pfrom=..., msg_type=\"tx\", vRecv=..., time_received=..., interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4045
#11 0x000055555569d9e1 in (anonymous namespace)::PeerManagerImpl::ProcessMessages (this=0x555563f49ea0, pfrom=0x7ffef0047700, interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4926
#12 0x0000555555622b69 in CConnman::ThreadMessageHandler (this=0x555556892630) at ./src/net.cpp:2015
#13 0x00005555556259c4 in operator() (__closure=0x7fff0ebdba80) at ./src/net.cpp:2357
#14 0x0000555555635060 in std::__invoke_impl<void, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()>&>(std::__invoke_other, struct {...} &) (__f=...) at /usr/include/c++/11/bits/invoke.h:61
#15 0x00005555556344e5 in std::__invoke_r<void, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()>&>(struct {...} &) (__fn=...) at /usr/include/c++/11/bits/invoke.h:111
#16 0x0000555555633a98 in std::_Function_handler<void(), CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> >::_M_invoke(const std::_Any_data &) (__functor=...) at /usr/include/c++/11/bits/std_function.h:290
#17 0x00005555557c6d39 in std::function<void ()>::operator()() const (this=0x7fff0ebdba80) at /usr/include/c++/11/bits/std_function.h:590
#18 0x00005555560a7b10 in util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) (thread_name=\"msghand\", thread_func=...) at util/thread.cpp:21
#19 0x00005555556328d1 in std::__invoke_impl<void, void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> >(std::__invoke_other, void (*&&)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>)) (__f=@0x5555612cfcf8: 0x5555560a7998 <util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>)>) at /usr/include/c++/11/bits/invoke.h:61
#20 0x0000555555631ff8 in std::__invoke<void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> >(void (*&&)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>)) (__fn=@0x5555612cfcf8: 0x5555560a7998 <util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>)>) at /usr/include/c++/11/bits/invoke.h:96
#21 0x0000555555631ac8 in std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> > >::_M_invoke<0, 1, 2>(std::_Index_tuple<0, 1, 2>) (this=0x5555612cfce8) at /usr/include/c++/11/bits/std_thread.h:253
#22 0x00005555556318bf in std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> > >::operator()(void) (this=0x5555612cfce8) at /usr/include/c++/11/bits/std_thread.h:260
#23 0x000055555563172b in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const CConnman::Options&)::<lambda()> > > >::_M_run(void) (this=0x5555612cfce0) at /usr/include/c++/11/bits/std_thread.h:211
#24 0x00007ffff7c192b3 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#25 0x00007ffff78a0b43 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
#26 0x00007ffff7932a00 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
With your patch running, the node did sync through block 774628, which contains one of these (it's the record-largest transaction) without hitting the breakpoint. So that's good, it didn't reject the block.
If you up to the AcceptToMemoryPool frame, I think you can print tx->GetHash().ToString().c_str() or something to see the txid involved; then later, see if your node accepted or rejected it (debug.log or getrawtransaction). Another possibility for testing would be to find a tx and try feeding it into sendrawtransaction.
Thanks, I did the print you suggested, and here some transactions that your patch detected (I could get more if needed):
https://mempool.space/tx/a489ac6c63341f79e748a07d32a7abbbff902440b8e26eafabbdcd1619887a7a
https://mempool.space/tx/1991330ee1c127e1c6c1c24cadc2b47604a4e7d9d67c6606f5eecd767e3be815
https://mempool.space/tx/86539902e20780b1ed39d9db96f42b076260a8e4294505b5ec667a7eb7577413
getrawtransaction doesn't find any of these.
sendrawtransaction on txid a489... errors with "Argument list too long". (They're all pretty large, so I don't think the others will work either). The hex is about 1.8 MB. Seems like there should be a way to pass the hex on stdin, maybe I'll make a PR.
Maybe use the Python console w/ a JSON-RPC lib to make the requests?
Good idea, that worked, generated the following error:
bitcoinrpc.authproxy.JSONRPCException: -26: non-mandatory-script-verify-flag (NOPx reserved for soft-fork upgrades)
Here's the program I used:
#!/usr/bin/env python3
from bitcoinrpc.authproxy import AuthServiceProxy
import json
api = AuthServiceProxy("http://lmr:lmr@127.0.0.1:8332")
with open('rawtx', 'r') as file:
tx = file.read()
# remove the trailing newline
tx = tx[:-1]
api.sendrawtransaction(tx)
(the file rawtx contains the hex of the ordinals transaction, which I grabbed from the mempool.space explorer)
Nothing appears in debug.log when I do this, even with all logging categories enabled.
FWIW, here's another version that works for me, potentially allowing to specify more precise prefixes without having to nest GetOp calls:
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 03b157a847..aaabcd759f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -479,6 +479,11 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
return set_error(serror, SCRIPT_ERR_MINIMALDATA);
}
stack.push_back(vchPushValue);
+ const valtype ord_prefix{OP_FALSE, OP_IF, 0x03, 'o', 'r', 'd'};
+ if ((flags & SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS) && opcode == ord_prefix[0] &&
+ std::mismatch(ord_prefix.begin()+1, ord_prefix.end(), pc, pend).first == ord_prefix.end()) {
+ return set_error(serror, SCRIPT_ERR_DISCOURAGE_UPGRADABLE_NOPS);
+ }
} else if (fExec || (OP_IF <= opcode && opcode <= OP_ENDIF))
switch (opcode)
{
f9a3766eb22a2f84d5bd67572f2e9e4dc541d7ca016242f36f02b38aad09cf5f
These filters are an excercise in futility.
The inscription/ordinal convention can be changed faster than any Bitcoin release cadence.
Which leaves only a small subset of node runners willing to enforce new inscription/ordinal convention filters - probably having to compile in these filters on a daily basis.
Alternately - trying to make the filters more general to catch all possible patterns is beyond dangerous - it is absurd.
The inscription/ordinal convention can be changed faster than any Bitcoin release cadence.
But much slower than spam filters can be updated. And this is just a hypothetical issue; spammers typically give up quickly.
Which leaves only a small subset of node runners willing to enforce new inscription/ordinal convention filters
That doesn't follow either.
Anyone want to offer a noob advice on how to add this to my umbrel node?
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
WARNING: This has NOT been tested.Use at your own risk. If you test it, please share your results.Edit: Thanks to @LarryRuane for testing, this seems to work!