bugfix, Change up submitpackage results to return results for all transactions #28848

pull instagibbs wants to merge 3 commits into bitcoin:master from instagibbs:2023-11-submitpackage-results changing 4 files +79 −32
  1. instagibbs commented at 9:10 pm on November 10, 2023: member

    This was prompted by errors being returned that didn’t “make any sense” to me, because it would for example return a “fee too low” error, when the “real” error was the child had something invalid, which disallowed CPFP evaluation. Rather than make judgment calls on what error is important(which is currently just return the “first”!), we simply return all errors and let the callers determine what’s best.

    Added a top level package_msg for quick eye-balling of general success of the package.

    This PR also fixes a couple bugs:

    1. Currently we don’t actually broadcast a transaction, even if it was entered into our mempool, if a subsequent transaction causes PKG_TX failure.
    2. “other-wtxid” is uncovered by tests, but IIUC was previously required to return “fees” and “vsize” results, but did not. I just make those results optional.
  2. DrahtBot commented at 9:10 pm on November 10, 2023: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK glozow, Sjors, achow101
    Stale ACK darosior

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #28950 (RPC: Add maxfeerate and maxburnamount args to submitpackage by instagibbs)
    • #27591 (rpc: distinguish between vsize and sigop-adjusted mempool vsize by glozow)

    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.

  3. instagibbs renamed this:
    Change up submitpackage results
    Change up submitpackage results to return results for all transactions
    on Nov 10, 2023
  4. instagibbs renamed this:
    Change up submitpackage results to return results for all transactions
    bugfix, Change up submitpackage results to return results for all transactions
    on Nov 10, 2023
  5. instagibbs force-pushed on Nov 10, 2023
  6. DrahtBot added the label CI failed on Nov 10, 2023
  7. DrahtBot removed the label CI failed on Nov 10, 2023
  8. instagibbs force-pushed on Nov 10, 2023
  9. instagibbs force-pushed on Nov 10, 2023
  10. DrahtBot added the label CI failed on Nov 10, 2023
  11. DrahtBot removed the label CI failed on Nov 10, 2023
  12. in src/rpc/mempool.cpp:856 in e77b702e3b outdated
    852                             }},
    853                         }},
    854+                        {RPCResult::Type::STR, "error", /*optional=*/true, "The transaction error string, if it was rejected by the mempool"},
    855                     }}
    856                 }},
    857                 {RPCResult::Type::ARR, "replaced-transactions", /*optional=*/true, "List of txids of replaced transactions",
    


    glozow commented at 9:25 am on November 11, 2023:
    Maybe add an “success” field for the full package, for convenience? Otherwise the user has to iterate through all the results to find out whether everything got submitted (which is likely the most common result in practice?)

    instagibbs commented at 3:17 pm on November 13, 2023:

    added a top-level “package-msg” response which is “success” when everything goes through.

    I also changed the logic to allow PCKG_POLICY to continue if there are transaction results, because this can happen if single transactions make it in, then a subsequent subpackage hits chain limits and is rejected.


    instagibbs commented at 3:17 pm on November 13, 2023:
    I will squash if/when we decide this is a good direction

    glozow commented at 3:49 pm on November 13, 2023:
    Makes sense to me. I’m in favor of returning all the information that we can to the user
  13. instagibbs force-pushed on Nov 13, 2023
  14. in src/rpc/mempool.cpp:900 in ddd9224f09 outdated
    900-                case PackageValidationResult::PCKG_POLICY:
    901+                case PackageValidationResult::PCKG_RESULT_UNSET:
    902                 {
    903-                    throw JSONRPCTransactionError(TransactionError::INVALID_PACKAGE,
    904-                        package_result.m_state.GetRejectReason());
    905+                    // Everything in the package should be submitted
    


    glozow commented at 4:23 pm on November 13, 2023:
    Comment says “Everything in the package should be submitted” but you’re only checking that the results exist in m_tx_results. Do you want to check that mempool.exists(wtxid) as well?

    instagibbs commented at 6:11 pm on November 13, 2023:
    I think the check would be for Txid since different witness isn’t considered a failure? added

    glozow commented at 6:34 pm on November 13, 2023:
    yeah makes sense
  15. in src/rpc/mempool.cpp:926 in ddd9224f09 outdated
    944             }
    945+
    946             size_t num_broadcast{0};
    947             for (const auto& tx : txns) {
    948+                // We don't want to re-submit the txn for validation in BroadcastTransaction
    949+                if (!mempool.exists(GenTxid::Txid(tx->GetHash()))) {
    


    glozow commented at 4:27 pm on November 13, 2023:
    Is using txid intentional here? If you use txid, BroadcastTransaction will rebroadcast the mempool tx if the result was DIFFERENT_WITNESS. If you use wtxid, we skip it.

    instagibbs commented at 6:11 pm on November 13, 2023:

    From my understanding: The current behavior in master is to re-broadcast anything in our mempool provided the txid matches something in the proposed package. If something in our mempool matches the txid, we (re)BroadcastTransaction. If it’s not in our mempool, it means it was rejected, and we don’t want to re-validate one by one.

    I think this is the behavior we want.


    glozow commented at 5:01 pm on November 27, 2023:
    Ah true, agree
  16. in src/rpc/mempool.cpp:930 in ddd9224f09 outdated
    948+                // We don't want to re-submit the txn for validation in BroadcastTransaction
    949+                if (!mempool.exists(GenTxid::Txid(tx->GetHash()))) {
    950+                    continue;
    951+                }
    952+
    953+                // We do not expect an error here; we are only broadcasting things already/still in mempool
    


    glozow commented at 4:31 pm on November 13, 2023:
    “all transactions were submitted” is not necessarily true anymore, no?

    instagibbs commented at 6:11 pm on November 13, 2023:
    fixed
  17. in src/rpc/mempool.cpp:893 in ddd9224f09 outdated
    889@@ -888,34 +890,57 @@ static RPCHelpMan submitpackage()
    890             Chainstate& chainstate = EnsureChainman(node).ActiveChainstate();
    891             const auto package_result = WITH_LOCK(::cs_main, return ProcessNewPackage(chainstate, mempool, txns, /*test_accept=*/ false));
    892 
    893-            // First catch any errors.
    894+            std::map<Wtxid, std::optional<std::string>> wtixd_to_error_strings;
    


    glozow commented at 4:38 pm on November 13, 2023:

    wtxid

    0            std::map<Wtxid, std::optional<std::string>> wtxid_to_error_strings;
    

    glozow commented at 4:38 pm on November 13, 2023:
    also, why have this intermediate map? can’t you do this in the loop at the end that populates rpc_result?

    instagibbs commented at 6:11 pm on November 13, 2023:
    removed the cruft
  18. in src/rpc/mempool.cpp:839 in ddd9224f09 outdated
    835@@ -836,19 +836,21 @@ static RPCHelpMan submitpackage()
    836         RPCResult{
    837             RPCResult::Type::OBJ, "", "",
    838             {
    839+                {RPCResult::Type::STR, "package-msg", "The transaction package result message. \"success\" indicates all transactions were accepted into or are already in the mempool."},
    


    glozow commented at 4:49 pm on November 13, 2023:
    Prefer using underscores #27501 (review)

    instagibbs commented at 6:11 pm on November 13, 2023:
    done
  19. instagibbs force-pushed on Nov 13, 2023
  20. DrahtBot added the label CI failed on Nov 13, 2023
  21. DrahtBot removed the label CI failed on Nov 13, 2023
  22. in src/rpc/mempool.cpp:825 in d72aab3dbd outdated
    821@@ -822,7 +822,7 @@ static RPCHelpMan submitpackage()
    822     return RPCHelpMan{"submitpackage",
    823         "Submit a package of raw transactions (serialized, hex-encoded) to local node.\n"
    824         "The package must consist of a child with its parents, and none of the parents may depend on one another.\n"
    825-        "The package will be validated according to consensus and mempool policy rules. If all transactions pass, they will be accepted to mempool.\n"
    826+        "The package will be validated according to consensus and mempool policy rules. If a transaction passes, it will be accepted to mempool.\n"
    


    Sjors commented at 11:42 am on November 23, 2023:
    a -> any ?

    Sjors commented at 11:52 am on November 23, 2023:
    Maybe add: “A transaction that’s already in the mempool is [ignored, rebroadcast?].”

    instagibbs commented at 4:21 pm on November 27, 2023:

    a -> any ?

    taken

    Maybe add: “A transaction that’s already in the mempool is [ignored, rebroadcast?].”

    Do we want to promise that? I’m ambivalent for now.

  23. in src/rpc/mempool.cpp:824 in d72aab3dbd outdated
    821@@ -822,7 +822,7 @@ static RPCHelpMan submitpackage()
    822     return RPCHelpMan{"submitpackage",
    823         "Submit a package of raw transactions (serialized, hex-encoded) to local node.\n"
    824         "The package must consist of a child with its parents, and none of the parents may depend on one another.\n"
    


    Sjors commented at 11:49 am on November 23, 2023:
    consist of parents followed by exactly one child,

    instagibbs commented at 4:21 pm on November 27, 2023:
    leaving un-touched, I think current text is correct
  24. Sjors commented at 12:26 pm on November 23, 2023: member

    Mmm, excuse my ignorance, but shouldn’t the minimum relay fee be ignored for packages? (Update: I see this is documented: https://github.com/bitcoin/bitcoin/blob/master/doc/policy/packages.md#package-fees-and-feerate, perhaps we could introduce a (test-only) startup argument to allow it? I’ll bring this up in #26933)

    I created two transactions while offline, wiping the mempool afterwards. To simulate a busy mempool, I set the -minrelaytxfee to 100, giving the parent a little bit less, and the child a little more than the minimum.

    0src/bitcoind -signet -minrelaytxfee=0.00100
    1src/bitcoin-cli -signet submitpackage '["02000000000101f7307b92590b1a7d82e878258a62ffaad05d0db510e947c3c331fe7400469cac0000000000fdffffff029d0c05020000000016001453dcf718842726f5236d0c05b1a5d43e12097731a086010000000000160014acdaaab857365e45724557f835694bdbb0c81231014009e5f00ec66eb80f497b771e2eb10cff222c60dc6b1fe865e5371142b1a02af7fbef913b0bc3bd49be679240e3c87bcde58532e53f657f9805352a43da036c965d9a0200", "02000000000101806ee5976b955c424940808f784fe92f746915a4d875dce994f3f72d8c8529aa0000000000fdffffff021027000000000000160014acdaaab857365e45724557f835694bdbb0c812317da7040200000000160014ef9af76b063593e215f4b861b61944b2e27624f60247304402200ac3df7c7e29df622a487ff075b8d224843e06f0848e16b2f02c55f9210eff9c0220133366f78a0191a74efbc348d2269326b6e27e11b885dd5b77a011de8a357fa00121031526c3e43061b7c9259695e75c512f5456b2dcb66ee475ece29b36688b1c35785d9a0200"]'
    
     0{
     1  "package_msg": "transaction failed",
     2  "tx-results": {
     3    "43bb567b00ba2d4424441af6702f841f0953e7aadd18adb475a84d5986186199": {
     4      "txid": "aa29858c2df7f394e9dc75d8a41569742fe94f788f804049425c956b97e56e80",
     5      "error": "min relay fee not met, 12838 < 13000"
     6    },
     7    "dff806dff6456a11d8cd2318615ae5234a4e4d60662731fc39db2857472af4c9": {
     8      "txid": "930455a47e5339e702cab0c435172a5497ff3af6d160d16224c9e9ad08cd8592",
     9      "error": "bad-txns-inputs-missingorspent"
    10    }
    11  },
    12  "replaced-transactions": [
    13  ]
    14}
    

    Also why is tx-results not an array like the input? (also to make it easier to sanity check that I didn’t submit them the wrong way around)

    (I can see how, for a script that submits transaction packages, this dictionary makes it easier to get feedback for specific transactions. For manual submission and debugging it’s easier to keep the same ordering, but this may not be a common use case.)

  25. instagibbs commented at 2:19 pm on November 23, 2023: member
    @Sjors I think you can set -minrelaytxfee=0 to work around it?
  26. Sjors commented at 2:56 pm on November 23, 2023: member

    @Sjors I think you can set -minrelaytxfee=0 to work around it?

    That doesn’t simulate a full mempool though.

    I wrote a commit that lets my example go through: https://github.com/Sjors/bitcoin/commit/03baacd7643daf3d1c8efbc1a723719d8cc72dce

    However, not sure if it’s worth the extra complexity.

  27. in test/functional/mempool_limit.py:209 in d72aab3dbd outdated
    205@@ -205,7 +206,7 @@ def test_mid_package_eviction(self):
    206 
    207         # Package should be submitted, temporarily exceeding maxmempool, and then evicted.
    208         with node.assert_debug_log(expected_msgs=["rolling minimum fee bumped"]):
    209-            assert_raises_rpc_error(-26, "mempool full", node.submitpackage, package_hex)
    210+            node.submitpackage(package_hex)
    


    Sjors commented at 3:10 pm on November 23, 2023:
    Do you still want to check “mempool full” like below?

    instagibbs commented at 4:21 pm on November 27, 2023:
    the check itself is a little weird, so I just double-checked that the package failed. The later checks should check invariants.
  28. Sjors commented at 3:21 pm on November 23, 2023: member

    Anyway, my two main issues are about pre-existing behavior: tx-results was already a dictionary and -minrelaytxfee already behaved that way.

    d72aab3dbd8847e851f8b724b195181e30e7bf02 looks good, but I’m (obviously) not familiar enough with mempool stuff to know for sure.

  29. instagibbs force-pushed on Nov 27, 2023
  30. instagibbs commented at 4:21 pm on November 27, 2023: member

    Also why is tx-results not an array like the input? (also to make it easier to sanity check that I didn’t submit them the wrong way around)

    If they’re submitted “backwards” they’ll be rejected.

    (I can see how, for a script that submits transaction packages, this dictionary makes it easier to get feedback for specific transactions. For manual submission and debugging it’s easier to keep the same ordering, but this may not be a common use case.)

    Right, I think this is 99%+ of the use-cases. “typical” usage of manual transaction creation should generate transactions with sufficient fees for an attempt at inclusion in a block.

  31. instagibbs commented at 5:09 pm on November 27, 2023: member
    all feedback is resolved
  32. Sjors commented at 1:42 pm on November 28, 2023: member
    ACK 4fb3340a1ac678165e6821645e6aa1e06f34921a
  33. in test/functional/rpc_packages.py:339 in 4fb3340a1a outdated
    334@@ -334,8 +335,10 @@ def test_submitpackage(self):
    335 
    336         self.log.info("Submitpackage only allows packages of 1 child with its parents")
    337         # Chain of 3 transactions has too many generations
    338+        legacy_pool = node.getrawmempool()
    339         chain_hex = [t["hex"] for t in self.wallet.create_self_transfer_chain(chain_length=25)]
    


    darosior commented at 8:26 am on November 29, 2023:
    Side note: should it be a chain of 3 transactions?

    instagibbs commented at 2:52 pm on November 29, 2023:
    it means “at least 3”, but you’re right this is probably too permissive a check. Not touching for now but can if people want me to.

    glozow commented at 3:22 pm on November 29, 2023:
    would be nice to fix, maybe in a followup
  34. darosior commented at 11:53 am on November 29, 2023: member

    Concept ACK

    Currently we don’t actually broadcast a transaction, even if it was entered into our mempool, if a subsequent transaction causes PKG_TX failure.

    Let’s have a test for this?

     0diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
     1index 6507f6267b..5ae805ccf0 100755
     2--- a/test/functional/rpc_packages.py
     3+++ b/test/functional/rpc_packages.py
     4@@ -340,6 +340,20 @@ class RPCPackagesTest(BitcoinTestFramework):
     5         assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, chain_hex)
     6         assert_equal(legacy_pool, node.getrawmempool())
     7 
     8+        # Create a transaction chain such as only the parent gets accepted (by making the child's
     9+        # version non-standard). Make sure the parent does get broadcast.
    10+        self.log.info("If a package is partially submitted, transactions included in mempool get broadcast")
    11+        peer = node.add_p2p_connection(P2PTxInvStore())
    12+        txs = self.wallet.create_self_transfer_chain(chain_length=2)
    13+        hex_partial_acceptance = [txs[0]["hex"], "f" + txs[1]["hex"][1:]]
    14+        res = node.submitpackage(hex_partial_acceptance)
    15+        assert_equal(res["package_msg"], "transaction failed")
    16+        first_wtxid = txs[0]["tx"].getwtxid()
    17+        assert "error" not in res["tx-results"][first_wtxid]
    18+        sec_wtxid = next(wtxid for wtxid in res["tx-results"] if wtxid != first_wtxid)
    19+        assert_equal(res["tx-results"][sec_wtxid]["error"], "version")
    20+        peer.wait_for_broadcast([first_wtxid])
    21+
    22 
    23 if __name__ == "__main__":
    24     RPCPackagesTest().main()
    

    The equivalent on master wouldn’t pass:

     0diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py
     1index 5644a9f5a8..d64c30bfe0 100755
     2--- a/test/functional/rpc_packages.py
     3+++ b/test/functional/rpc_packages.py
     4@@ -337,6 +337,12 @@ class RPCPackagesTest(BitcoinTestFramework):
     5         chain_hex = [t["hex"] for t in self.wallet.create_self_transfer_chain(chain_length=25)]
     6         assert_raises_rpc_error(-25, "package topology disallowed", node.submitpackage, chain_hex)
     7 
     8+        peer = node.add_p2p_connection(P2PTxInvStore())
     9+        txs = self.wallet.create_self_transfer_chain(chain_length=2)
    10+        hex_partial_acceptance = [txs[0]["hex"], "f" + txs[1]["hex"][1:]]
    11+        assert_raises_rpc_error(-26, "version", node.submitpackage, hex_partial_acceptance)
    12+        peer.wait_for_broadcast([txs[0]["tx"].getwtxid()])
    13+
    14 
    15 if __name__ == "__main__":
    16     RPCPackagesTest().main()
    
  35. darosior commented at 12:28 pm on November 29, 2023: member

    ACK 4fb3340a1ac678165e6821645e6aa1e06f34921a

    I considered suggesting to backport this at first, in light of #27609. But i don’t think the fix for relaying the parent transaction if the child fails validation matters much for this usecase (mining pool calling submitpackage). At least, not enough to warrant a backport this late i’d say.

  36. instagibbs commented at 2:51 pm on November 29, 2023: member
    @darosior thanks, took the test with minor modifications to not do hex-surgery :)
  37. darosior commented at 2:54 pm on November 29, 2023: member
    much-less-fun ACK 24c345f89ecad54223e42f8cf084ef3c5ed5f0c0
  38. DrahtBot requested review from Sjors on Nov 29, 2023
  39. in src/rpc/mempool.cpp:845 in 4fb3340a1a outdated
    842                     {RPCResult::Type::OBJ, "wtxid", "transaction wtxid", {
    843                         {RPCResult::Type::STR_HEX, "txid", "The transaction hash in hex"},
    844                         {RPCResult::Type::STR_HEX, "other-wtxid", /*optional=*/true, "The wtxid of a different transaction with the same txid but different witness found in the mempool. This means the submitted transaction was ignored."},
    845-                        {RPCResult::Type::NUM, "vsize", "Virtual transaction size as defined in BIP 141."},
    846-                        {RPCResult::Type::OBJ, "fees", "Transaction fees", {
    847+                        {RPCResult::Type::NUM, "vsize", /*optional=*/true, "Virtual transaction size as defined in BIP 141."},
    


    glozow commented at 2:58 pm on November 29, 2023:
    Not changed here but I think the doc should actually say sigop-adjusted virtual size, no?

    instagibbs commented at 4:32 pm on November 29, 2023:
    pushed a fixup commit on top
  40. in src/rpc/mempool.cpp:981 in 4fb3340a1a outdated
    976@@ -959,6 +977,8 @@ static RPCHelpMan submitpackage()
    977                             replaced_txids.insert(ptx->GetHash());
    978                         }
    979                     }
    980+                } else {
    981+                    NONFATAL_UNREACHABLE();
    


    glozow commented at 3:02 pm on November 29, 2023:
    nit/followup: This could instead be a compile-time check if the if/else blocks are converted to a switch statement on result type

    instagibbs commented at 4:32 pm on November 29, 2023:
    done
  41. in test/functional/mempool_limit.py:375 in 4fb3340a1a outdated
    369@@ -366,7 +370,9 @@ def run_test(self):
    370         assert_greater_than(worst_feerate_btcvb, (parent_fee + child_fee) / (tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize()))
    371         assert_greater_than(mempoolmin_feerate, (parent_fee) / (tx_parent_just_below["tx"].get_vsize()))
    372         assert_greater_than((parent_fee + child_fee) / (tx_parent_just_below["tx"].get_vsize() + tx_child_just_above["tx"].get_vsize()), mempoolmin_feerate / 1000)
    373-        assert_raises_rpc_error(-26, "mempool full", node.submitpackage, [tx_parent_just_below["hex"], tx_child_just_above["hex"]])
    374+        res = node.submitpackage([tx_parent_just_below["hex"], tx_child_just_above["hex"]])
    375+        for wtxid in [tx_parent_just_below["wtxid"], tx_child_just_above["wtxid"]]:
    376+            res["tx-results"][wtxid]["error"] == "mempool full"
    


    glozow commented at 3:06 pm on November 29, 2023:

    Are we trying to assert that this is true? Pretty sure this just keeps going if the expression is False

    0            assert_equal(res["tx-results"][wtxid]["error"], "mempool full")
    

    instagibbs commented at 4:32 pm on November 29, 2023:
    oops, taken
  42. in src/rpc/mempool.cpp:916 in 4fb3340a1a outdated
    925-                        }
    926-                    }
    927-                    // If a PCKG_TX error was returned, there must have been an invalid transaction.
    928-                    NONFATAL_UNREACHABLE();
    929+                    // Package-wide error we want to return, but we also want to return individual responses
    930+                    package_msg = package_result.m_state.GetRejectReason();
    


    glozow commented at 3:12 pm on November 29, 2023:
    nit: can also CHECK_NONFATAL(package_result.m_tx_results.size() == txns.size()); here

    instagibbs commented at 4:32 pm on November 29, 2023:
    taken

    instagibbs commented at 5:57 pm on November 29, 2023:
    actually this is incorrect; PCKG_POLICY is grouped here, which means it’s size of txns or 0. Updated the check
  43. in test/functional/rpc_packages.py:355 in 24c345f89e outdated
    350+        hex_partial_acceptance = [txs[0]["hex"], bad_child.serialize().hex()]
    351+        res = node.submitpackage(hex_partial_acceptance)
    352+        assert_equal(res["package_msg"], "transaction failed")
    353+        first_wtxid = txs[0]["tx"].getwtxid()
    354+        assert "error" not in res["tx-results"][first_wtxid]
    355+        sec_wtxid = next(wtxid for wtxid in res["tx-results"] if wtxid != first_wtxid)
    


    glozow commented at 3:18 pm on November 29, 2023:
    can’t you just do sec_wtxid = bad_child.getwtxid()?

    instagibbs commented at 4:32 pm on November 29, 2023:
    taken
  44. in src/rpc/mempool.cpp:952 in 4fb3340a1a outdated
    964+                if (it == package_result.m_tx_results.end()) {
    965+                    // No results, report error and continue
    966+                    result_inner.pushKV("error", "unevaluated");
    967+                    continue;
    968+                }
    969+                CHECK_NONFATAL(it != package_result.m_tx_results.end());
    


    glozow commented at 3:21 pm on November 29, 2023:
    This check is useless? It exits if untrue right before

    instagibbs commented at 4:32 pm on November 29, 2023:
    removed
  45. glozow commented at 3:24 pm on November 29, 2023: member
    4fb3340a1ac678165e6821645e6aa1e06f34921a interface LGTM, some small code errors
  46. instagibbs force-pushed on Nov 29, 2023
  47. instagibbs commented at 4:32 pm on November 29, 2023: member

    would be nice to fix, maybe in a followup

    took the liberty of doing now @glozow

  48. glozow commented at 4:38 pm on November 29, 2023: member
    ACK 24953124a6ca70322c56f40ac090af174a3f4553
  49. DrahtBot requested review from darosior on Nov 29, 2023
  50. RPC submitpackage: change return format to allow partial errors
    Behavior prior to this commit allows some transactions to
    enter into the local mempool but not be reported to the user
    when encountering a PackageValidationResult::PCKG_TX result.
    
    This is further compounded with the fact that any transactions
    submitted to the mempool during this call would also not be
    relayed to peers, resulting in unexpected behavior.
    
    Fix this by, if encountering a package error, reporting all
    wtxids, along with a new error field, and broadcasting every
    transaction that was found in the mempool after submission.
    
    Note that this also changes fees and vsize to optional,
    which should also remove an issue with other-wtxid cases.
    b67db52c39
  51. doc: submitpackage vsize results are sigops-adjusted e67a345162
  52. test_submitpackage: only make a chain of 3 txns f23ba24aa0
  53. instagibbs force-pushed on Nov 29, 2023
  54. DrahtBot added the label CI failed on Nov 29, 2023
  55. instagibbs commented at 5:57 pm on November 29, 2023: member
    fixed CI fuzz rpc corpus issue with relaxed check
  56. DrahtBot removed the label CI failed on Nov 29, 2023
  57. instagibbs commented at 2:30 pm on November 30, 2023: member
    ping @Sjors @darosior @glozow should be ready for re-acks
  58. glozow commented at 2:13 pm on December 1, 2023: member
    utACK f23ba24aa079d68697d475789cd21bd7b5075550, thanks for taking the suggestions
  59. Sjors commented at 3:12 pm on December 1, 2023: member
    Light re-utACK f23ba24aa079d68697d475789cd21bd7b5075550
  60. DrahtBot removed review request from Sjors on Dec 1, 2023
  61. achow101 commented at 5:09 pm on December 1, 2023: member
    ACK f23ba24aa079d68697d475789cd21bd7b5075550
  62. achow101 merged this on Dec 1, 2023
  63. achow101 closed this on Dec 1, 2023


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-01 10:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me