test: add unit test for non-standard txs with too large tx size #17947

1. 
   theStack commented at 11:15 am on January 17, 2020: member

   Approaches another missing unit test of issue #17394: Checks that the function IsStandardTx() returns rejection reason "tx-size" if the transaction weight is larger than MAX_STANDARD_TX_WEIGHT (=400000 vbytes).
2. test: add unit test for non-standard txs with too large tx size

   The function IsStandardTx() returns rejection reason "tx-size" if the
   transaction weight is larger than MAX_STANDARD_TX_WEIGHT (=400000 vbytes).

   4537ba5f21
3. fanquake added the label Tests on Jan 17, 2020
4. fanquake requested review from instagibbs on Jan 17, 2020
5. 
   theStack commented at 11:18 am on January 17, 2020: member

   Note that there is still an open PR #17570 for this unit test but the author hasn’t been active for more than five weeks now.
6. 
   DrahtBot commented at 2:58 pm on January 17, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #17720 (test: add unit test for non-standard “scriptsig-not-pushonly” txs by theStack)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

7. 
   Empact commented at 7:34 pm on January 17, 2020: member

   Code Review ACK https://github.com/bitcoin/bitcoin/pull/17947/commits/4537ba5f21ad8afb705325cd8e15dd43877eb28f
8. in src/test/transaction_tests.cpp:828 in 4537ba5f21

   820@@ -821,9 +821,29 @@ BOOST_AUTO_TEST_CASE(test_IsStandard)
   821     BOOST_CHECK(!IsStandardTx(CTransaction(t), reason));
   822     BOOST_CHECK_EQUAL(reason, "scriptsig-size");
   823 
   824+    // Check tx-size (non-standard if transaction weight is > MAX_STANDARD_TX_WEIGHT)
   825+    t.vin.clear();
   826+    t.vin.resize(2438); // size per input (empty scriptSig): 41 bytes
   827+    t.vout[0].scriptPubKey = CScript() << OP_RETURN << std::vector<unsigned char>(19, 0); // output size: 30 bytes
   828+    // tx header:                12 bytes =>     48 vbytes
   

   ---

   ---

   

   instagibbs commented at 8:41 pm on January 17, 2020:

   micro-nit: I don’t think this big block of text is necessary for the test reader. We trust that GetTransactionWeight does the right thing, and checking that IsStandardTx does what we expect.

   ---

   

   instagibbs commented at 8:42 pm on January 17, 2020:

   no need to invalidate ACKs for this, just noting!

   ---

   

   theStack commented at 7:32 pm on January 18, 2020:

   The idea was to reason how we end up magically from two numbers (2438 and 19) to a transaction size with exactly 400000 vbytes. In case this needs to be altered in the future to reach another tx weight, a test writer has it easier to figure out what to change (I admit that it is very unlikely to ever happen). I agree that it could be a bit shorter though.
9. 
   instagibbs commented at 8:42 pm on January 17, 2020: member

   ACK https://github.com/bitcoin/bitcoin/commit/4537ba5f21ad8afb705325cd8e15dd43877eb28f
10. 
    theStack commented at 11:27 am on February 7, 2020: member

    Since there has been no activity for three weeks: is there anything else I can do for this PR?
11. laanwj referenced this in commit ceb3d45f7d on Feb 10, 2020
12. laanwj merged this on Feb 10, 2020
13. laanwj closed this on Feb 10, 2020

    ---

14. sidhujag referenced this in commit a8cc03f1fe on Feb 18, 2020
15. sidhujag referenced this in commit b63e90ed86 on Nov 10, 2020
16. theStack deleted the branch on Dec 1, 2020
17. DrahtBot locked this on Feb 15, 2022

Contributors
theStack DrahtBot Empact instagibbs

Labels
Tests