CBasicKeyStore::AddCScript() : redeemScripts > 520 bytes are invalid #4313

The extra sanity checking PeterTodd added a while back causes us to crap out while loading wallets which have previously had an excessively large addmultisigaddress done to them.

Do you have a backtrace of the error that happens? I'm not sure how it ends up in that check, in principle loading the wallet shouldn't fail on transactions.

I just threw a breakpoint on AddCScript—

#0 CBasicKeyStore::AddCScript (this=this@entry=0x55557602e5f0, redeemScript=...) at keystore.cpp:35 #1 0x0000555555790c81 in LoadCScript (redeemScript=..., this=0x55557602e5f0) at wallet.h:214 #2 ReadKeyValue (pwallet=pwallet@entry=0x55557602e5f0, ssKey=..., ssValue=..., wss=..., strType="cscript", strErr="") at walletdb.cpp:547 #3 0x000055555579154b in CWalletDB::LoadWallet (this=this@entry=0x7fffffffbf40, pwallet=pwallet@entry=0x55557602e5f0) at walletdb.cpp:623 #4 0x000055555577b9ee in CWallet::LoadWallet (this=0x55557602e5f0, fFirstRunRet=@0x7fffffffcaa0: false) at wallet.cpp:1475 #5 0x00005555555a5a4b in AppInit2 (threadGroup=...) at init.cpp:975 #6 0x000055555558d513 in AppInit (argc=argc@entry=2, argv=argv@entry=0x7fffffffdd28) at bitcoind.cpp:143 #7 0x000055555558381f in main (argc=2, argv=0x7fffffffdd28) at bitcoind.cpp:180

Thanks. I see. This happens while reading the "cscript" record, not while reading a transaction, that's why I couldn't find it.

What would the right behaviour here?

• Should we just pretend that these CScripts are not there when loading from an existing wallet (with a warning)?
• Or should they be force-added, bypassing the check? (this could result in problems later, so I prefer the above)

I think the first. Just ignore them, maybe a warning in the logs. (though we may get complaints that there is a warning with no way to cure it…)

Well we could define a new wallet version, ignore them only in the current wallet version (and warn), and make an upgrader that removes them. That would provide an upgrade path to get rid of the warnings.

But that may be too much. Bumping the wallet version would break compatibility with old wallets for no good reason, for example.