test: added test to assert TX decode rpc error on submitpackage rpc #31139

two empty string might be simpler?

thanks! fair point updated in 4bdb78c

While I was going through this test after I checked out the branch in local, I realised that this assertion seems a bit out of context because of the comment on line 378. How about adding this assertion before line 378?

Thanks! I moved it more towards the start of the function in 34c6643

I don't think just passing two empty strings is the right way of testing it because:

• it only plays with non-hex values and do not exercise the hex parsing.
• if the first tx is invalid, the second one has no effect.

Ya that makes sense!

What I did in 34c6643

Is I used a proper hex value and then had the second value malformed so that we can cover more bases

let me know if that looks good to you I can modify it other ways aswell!

it only plays with non-hex values and do not exercise the hex parsing.

an empty string is valid hex, it's just an invalid tx, no? If you want to test invalid hex that's another thing.

if the first tx is invalid, the second one has no effect.

You need two transactions currently for it to get far enough to try and deserialize. I don't really care which one(s) are valid except for ease of reading.

let me know if that looks good to you I can modify it other ways aswell!

imo having valid hex is a more realistic scenario, but coverage is coverage

would be good to double-check that the first tx doesn't make it into the mempool even if it otherwise would

Agree, having this assertion would be good because the valid tx would still not make to mempool in this case.

ok I added an assert to check for txid in getrawmempool in 1fab1d6

let me know if that looks good to you!

Are these blank lines intentional?

I removed them in 1fab1d6

        assert txid_list[0] not in node.getrawmempool()
        assert txid_list[1] not in node.getrawmempool()

Thanks!

Updated in d7fd766feb2f579bdba0e778bacdeb13103e8282

Induced failure with:

-        hex_list = [valid_tx_list[0]["hex"][:-1] + 'X', valid_tx_list[1]["hex"]]
+        hex_list = [valid_tx_list[0]["hex"], valid_tx_list[1]["hex"]]

Also verified that the not in node.getrawmempool() asserts also fail, e.g.

         self.log.info("Submitpackage only allows valid hex inputs")
         valid_tx_list = self.wallet.create_self_transfer_chain(chain_length=2)
-        hex_list = [valid_tx_list[0]["hex"][:-1] + 'X', valid_tx_list[1]["hex"]]
+        hex_list = [valid_tx_list[0]["hex"], valid_tx_list[1]["hex"]]
         txid_list = [valid_tx_list[0]["txid"], valid_tx_list[1]["txid"]]
-        assert_raises_rpc_error(-22, "TX decode failed:", node.submitpackage, hex_list)
-        assert txid_list[0] not in node.getrawmempool()
+        node.submitpackage(hex_list)
+        #assert_raises_rpc_error(-22, "TX decode failed:", node.submitpackage, hex_list)
+        #assert txid_list[0] not in node.getrawmempool()
         assert txid_list[1] not in node.getrawmempool()