Skip to content

backport: bitcoin#23590, 23061, 23694#6256

Merged
PastaPastaPasta merged 3 commits into
dashpay:developfrom
vijaydasmp:bp23_2024_09_01
Oct 21, 2024
Merged

backport: bitcoin#23590, 23061, 23694#6256
PastaPastaPasta merged 3 commits into
dashpay:developfrom
vijaydasmp:bp23_2024_09_01

Conversation

@vijaydasmp

@vijaydasmp vijaydasmp commented Sep 7, 2024

Copy link
Copy Markdown

This backport contains following updates

  • Crash on Mempool ConsensusScriptChecks Failure: Added a fix to crash debug builds when ConsensusScriptChecks in the mempool fails. This prevents bugs from bypassing the testing infrastructure by forcing a crash during tests.
  • Fix for -persistmempool Option: Corrected the behavior of -persistmempool, which was previously interpreted as -nopersistmempool. This update ensures the flag works as intended.
  • Documentation: Added the missing "optional" field to the MempoolEntryDescription documentation for better clarity.

@vijaydasmp vijaydasmp changed the title backport: bitcoin# backport: bitcoin#23590, 23528, 2306 Sep 7, 2024
@vijaydasmp vijaydasmp changed the title backport: bitcoin#23590, 23528, 2306 backport: bitcoin#23590, 23528, 23061 Sep 7, 2024
@vijaydasmp vijaydasmp changed the title backport: bitcoin#23590, 23528, 23061 backport: bitcoin#23590, 23528, 23061, 23694 Sep 8, 2024
@vijaydasmp vijaydasmp changed the title backport: bitcoin#23590, 23528, 23061, 23694 backport: bitcoin#23590, 23258, 23061, 23694 Sep 8, 2024
@vijaydasmp
vijaydasmp marked this pull request as ready for review September 8, 2024 06:25
@vijaydasmp

Copy link
Copy Markdown
Author

Hello @UdjinM6 requesting review

UdjinM6
UdjinM6 previously approved these changes Sep 11, 2024

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 376cb957e8ce9b22e53ef40e73170933833ac7f5

@vijaydasmp

vijaydasmp commented Sep 12, 2024

Copy link
Copy Markdown
Author

Hello @PastaPastaPasta, @knst , requesting review

@UdjinM6 UdjinM6 added this to the 21.2 milestone Sep 22, 2024
Comment thread src/wallet/wallet.cpp Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

23258: nit: missing parenthesis

chain -> chain()

Comment thread doc/release-notes-23061.md Outdated
Comment on lines 1 to 5

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

23061: we don't need a release note for it, because this issue has been fixed very long time ago by kwvg here 18514d3 (#5546)

Please, keep only changes in test/functional/mempool_persist.py for this backport

@vijaydasmp

Copy link
Copy Markdown
Author

Hello @knst , requesting review

UdjinM6
UdjinM6 previously approved these changes Sep 27, 2024

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 65821b9c2f7660c5f77e515b9490f1fe202f9692

@knst knst left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 65821b9c2f7660c5f77e515b9490f1fe202f9692

knst
knst previously requested changes Oct 1, 2024

@knst knst left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got locally a crash (assert) on this code, I am checking more.

Logs:

validation.cpp:810 ConsensusScriptChecks: Assertion `false' failed.
Posix Signal: Aborted
   0#: (0x5E97E5A39494) stl_vector.h:115        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_copy_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data const&)
   1#: (0x5E97E5A39494) stl_vector.h:127        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_swap_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data&)
   2#: (0x5E97E5A39494) stl_vector.h:1959       - std::vector<unsigned long, std::allocator<unsigned long> >::_M_move_assign(std::vector<unsigned long, std::allocator<unsigned long> >&&, std::integral_constant<bool, true>)
   3#: (0x5E97E5A39494) stl_vector.h:768        - std::vector<unsigned long, std::allocator<unsigned long> >::operator=(std::vector<unsigned long, std::allocator<unsigned long> >&&)
   4#: (0x5E97E5A39494) stacktraces.cpp:777     - HandlePosixSignal
   5#: (0x7F1353442990) libc_sigaction.c        - ???
   6#: (0x7F1353499A1B) pthread_kill.c:44       - __pthread_kill_implementation
   7#: (0x7F1353499A1B) pthread_kill.c:78       - __pthread_kill_internal
   8#: (0x7F1353499A1B) pthread_kill.c:89       - __GI___pthread_kill
   9#: (0x7F13534428E6) raise.c:27              - __GI_raise
  10#: (0x7F13534268B7) abort.c:81              - __GI_abort
  11#: (0x5E97E519BD58) basic_string.h:792      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string()
  12#: (0x5E97E519BD58) check.cpp:14            - assertion_fail(char const*, int, char const*, char const*)
  13#: (0x5E97E55CB53A) validation.h:118        - TxValidationState::TxValidationState(TxValidationState const&)
  14#: (0x5E97E55CB53A) validation.cpp:883      - AcceptSingleTransaction
  15#: (0x5E97E55CB53A) validation.cpp:977      - AcceptToMemoryPoolWithTime
  16#: (0x5E97E55CC465) validation.cpp:999      - AcceptToMemoryPool(CChainState&, CTxMemPool&, std::shared_ptr<CTransaction const> const&, bool, bool)
  17#: (0x5E97E53BD299) transaction.cpp:70      - BroadcastTransaction(NodeContext&, std::shared_ptr<CTransaction const>, bilingual_str&, long const&, bool, bool, bool)
  18#: (0x5E97E5519F32) shared_ptr_base.h:1070  - std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count()
  19#: (0x5E97E5519F32) shared_ptr_base.h:1524  - std::__shared_ptr<CTransaction const, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr()
  20#: (0x5E97E5519F32) shared_ptr.h:175        - std::shared_ptr<CTransaction const>::~shared_ptr()
  21#: (0x5E97E5519F32) rawtransaction.cpp:1162 - operator()
  22#: (0x5E97E551A1E4) std_function.h:292      - _M_invoke
  23#: (0x5E97E59CE276) util.cpp:521            - RPCHelpMan::HandleRequest(JSONRPCRequest const&) const
  24#: (0x5E97E544C16A) univalue.h:17           - UniValue::operator=(UniValue&&)
  25#: (0x5E97E544C16A) server.h:108            - CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const
  26#: (0x5E97E554B9D4) std_function.h:591      - std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const
  27#: (0x5E97E554B9D4) server.cpp:622          - ExecuteCommand
  28#: (0x5E97E554CA7F) server.cpp:511          - ExecuteCommands
  29#: (0x5E97E554CA7F) server.cpp:543          - CRPCTable::execute(JSONRPCRequest const&) const

@vijaydasmp

vijaydasmp commented Oct 1, 2024

Copy link
Copy Markdown
Author

Hi @knst should I rebase it with develop?

@knst

knst commented Oct 1, 2024

Copy link
Copy Markdown
Collaborator

Hi @knst should I rebase it with develop?

yes, please rebase it with develop just to let everyone see this failure in CI without running own local copy to reproduce issue

I am investigating issue already, I will update about status later or maybe provide a fix

@UdjinM6

UdjinM6 commented Oct 1, 2024

Copy link
Copy Markdown

@knst pls check #6298

@vijaydasmp
vijaydasmp requested a review from knst October 2, 2024 10:31
PastaPastaPasta added a commit that referenced this pull request Oct 3, 2024
c91ba8a fix: no crashes allowed (UdjinM6)
a4cd1d6 fix: explicitly test no tx in mempool after invalidateblock (UdjinM6)
04b5db9 test: extend and refactor DIP0020 activation test (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  ~23590 backport in #6256 results in node crashes (for dashd compiled with `--enable-debug`) when we try to spend coins locked via disabled opcodes in `feature_dip0020_activation.py`. Also,~ there is only rpc part and no tests for relaying such txes via p2p.

  ## What was done?

  ## How Has This Been Tested?
  run test on develop and on top of #6256 + #6299 with and without `--enable-debug`

  ## Breaking Changes

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK c91ba8a
  knst:
    utACK c91ba8a

Tree-SHA512: 2ba16d6a6bb58cb98c01234ed60a8eecd4ff214d3d8386a4b8ed10f4776e0862d7794747791d82345d6031678a308df39c2dbdd361a902ee1e56cf7f05a73c1a
@knst

knst commented Oct 4, 2024

Copy link
Copy Markdown
Collaborator

I convert this one in Draft until one of these PRs are not merged: #6302 or #6299 @vijaydasmp

@knst
knst marked this pull request as draft October 4, 2024 09:09
@github-actions

github-actions Bot commented Oct 7, 2024

Copy link
Copy Markdown

This pull request has conflicts, please rebase.

PastaPastaPasta added a commit that referenced this pull request Oct 15, 2024
…ivated long time ago

c8fd37d docs: added a comment about removed SCRIPT_ENABLE_DIP0020_OPCODES (Konstantin Akimov)
61bc300 feat: drop SCRIPT_ENABLE_DIP0020_OPCODES, make opcodes available from genesis block (Konstantin Akimov)
0e55abd feat: remove feature_dip0020_activationl.py functional test and related code (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  it's alternate solution for #6299 to fix a crash reported #6256

  ## What was done?
  Removed code related to DIP0020 activation for various OP codes. DIP0020 is activated long time ago and no any historical blocks are violating rules, removing it's backwards compatible.

  ## How Has This Been Tested?
  Run unit and functional tests. See also changes in data for unit tests and removed functional test.
  It also re-index mainnet and testnet successfully
  ```
  src/qt/dash-qt  -reindex -assumevalid=0
  src/qt/dash-qt -testnet -reindex -assumevalid=0
  ```

  Also extra test is done with bitcoin#23590 - no crash with it in `feature_dip0020_activation.py` [modified assuming it is always activated]

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK c8fd37d
  PastaPastaPasta:
    utACK c8fd37d

Tree-SHA512: 05ddda4e8fb66305995e91c8a04fbda690aef8fb82acb23b7d62f302da60b5ec7e7a97bd988efd2523dbd9cafde9f4b65cae2db9e4b5257464ce1c8fcca6a40f
@knst

knst commented Oct 15, 2024

Copy link
Copy Markdown
Collaborator

@vijaydasmp #6302 is merged, please, rebase this PR to the top of develop

MarcoFalke added 3 commits October 17, 2024 20:19
…hecks fails

faad05c Crash debug builds when mempool ConsensusScriptChecks fails (MarcoFalke)

Pull request description:

  Currently a bug in the function might sneak around our testing infrastructure.

  Fix that by turning bugs into crashes during tests.

ACKs for top commit:
  glozow:
    utACK faad05c, there's something seriously wrong with the code if this returns false, good to throw in debug mode

Tree-SHA512: dfea1cd9ce3f1c303f49cca1417cd5c77c6ed12849aaff7b6ab1b6060f2f0c9cf5d4689017355d11f66639bab35823f65f848e6979042fa875181509dfd5d3d7
faa9c19 doc: Add 23061 release notes (MarcoFalke)
faff17b Fix (inverse) meaning of -persistmempool (MarcoFalke)

Pull request description:

  Passing `-persistmempool` is currently treated as `-nopersistmempool`

ACKs for top commit:
  jnewbery:
    reACK faa9c19
  hebasto:
    ACK faa9c19, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: f34a89a07745dabe340eb845b2a348b79c093e9056f7a21c17e1ba2e278177c9b4cf30e8095791fd645a7f90eb34850b2eee0c869b4f6ec02bf749c73b0e52ee
…tion

fa1571b doc: Add missing optional to MempoolEntryDescription (MarcoFalke)

Pull request description:

  Needed for bitcoin#23083.

  Can be reviewed with `--word-diff-regex=.`.

ACKs for top commit:
  josibake:
    ACK bitcoin@fa1571b
  shaavan:
    ACK fa1571b

Tree-SHA512: b4370003d2aeadce438778e15bd9a0d6a7fef4711acbe8471a50a9d72bbf74e1705fecbaae6f7eb367ece7c795a816c4b8b6583ed6c8f91b35621ca30fd95c18
@vijaydasmp vijaydasmp changed the title backport: bitcoin#23590, 23258, 23061, 23694 backport: bitcoin#23590, 23061, 23694 Oct 17, 2024
@vijaydasmp
vijaydasmp marked this pull request as ready for review October 17, 2024 16:32
@vijaydasmp
vijaydasmp requested a review from UdjinM6 October 18, 2024 04:11

@UdjinM6 UdjinM6 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 7d170bd

@vijaydasmp

Copy link
Copy Markdown
Author

Hello @knst @PastaPastaPasta , please review

@PastaPastaPasta PastaPastaPasta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 7d170bd

@PastaPastaPasta
PastaPastaPasta dismissed knst’s stale review October 21, 2024 14:20

should be fixed now

@PastaPastaPasta
PastaPastaPasta merged commit 40dc6d9 into dashpay:develop Oct 21, 2024
@UdjinM6 UdjinM6 modified the milestones: 21.2, 22 Oct 29, 2024
PastaPastaPasta added a commit that referenced this pull request Dec 30, 2024
, bitcoin#23804, bitcoin#24310, bitcoin#24537, bitcoin#24582, bitcoin#24404, bitcoin#25013, bitcoin#25029, bitcoin#25254, bitcoin#25388 (mempool backports)

578be36 merge bitcoin#25388: move policy constants to policy (Kittywhiskers Van Gogh)
aca2c0d merge bitcoin#25254: Move minRelayTxFee to policy/settings (Kittywhiskers Van Gogh)
1d82dd8 merge bitcoin#25029: Move fee estimation RPCs to separate file (Kittywhiskers Van Gogh)
ad94a30 merge bitcoin#25013: Remove cs_main from verifymessage, move msg utils to new file (Kittywhiskers Van Gogh)
8e42b12 merge bitcoin#24404: Remove confusing P1008R1 violation in ATMPArgs (Kittywhiskers Van Gogh)
006370d merge bitcoin#24582: Move txoutproof RPCs to txoutproof.cpp (Kittywhiskers Van Gogh)
dfa4772 merge bitcoin#24537: Split mempool RPCs from blockchain.cpp (Kittywhiskers Van Gogh)
537d3b3 merge bitcoin#24310: docs / fixups from RBF and packages (Kittywhiskers Van Gogh)
8239d5f merge bitcoin#23804: followups for de-duplication of packages (Kittywhiskers Van Gogh)
71b2623 merge bitcoin#22674: mempool validation and submission for packages of 1 child + parents (Kittywhiskers Van Gogh)
8392d23 merge bitcoin#23381: refactoring for package submission (Kittywhiskers Van Gogh)
d3c0059 merge bitcoin#22689: deprecate top-level fee fields in getmempool RPCs (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * [bitcoin#23694](bitcoin#23694) (backported in [dash#6256](#6256)) was backported before [bitcoin#22689](bitcoin#22689) and therefore, the formatting changes in the former were not incorporated in the backport. This has been resolved by accommodating them in the latter.

  * The fields deprecated in [bitcoin#22689](bitcoin#22689) were originally deprecated in [bitcoin#12240](bitcoin#12240) but not gated behind runtime arguments, the backport does so.

  * This pull request includes multiple backports that include code specific to RBF (BIP 125) and SegWit, which have been removed due to Dash Core not supporting them, comments and documentation (like `doc/policy/packages.md`) have been amended to reflect this accordingly.

  * When backporting [bitcoin#22674](bitcoin#22674), a new constructor was introduced for submitting `MEMPOOL_ENTRY` `ResultType`, upstream this can be done trivially as it uses function overloading ([source](https://github.com/bitcoin/bitcoin/pull/22674/files#diff-d3c243938494b10666b44404a27af7d84b44a72b85a27431e0c89e181462ca6eR198-R204)) but as Dash Core doesn't accept
  `replaced_txns`, there is no opportunity to differentiate arguments.
    * This was resolved by accepting `ResultType` as an argument but always passing `MEMPOOL_ENTRY` as the argument ([source](71b2623#diff-d3c243938494b10666b44404a27af7d84b44a72b85a27431e0c89e181462ca6eR232-R242))

  * Unit tests like `package_witness_swap_tests` (introduced in [bitcoin#23804](bitcoin#23804)) have been omitted due to functionality tested not being implemented in Dash Core. Subsequent backports that include modifications to these tests have mirrored similar omissions.

  ## Breaking Changes

  * The top-level fee fields `fee`, `modifiedfee`, `ancestorfees` and `descendantfees` returned by RPCs `getmempoolentry`,`getrawmempool(verbose=true)`, `getmempoolancestors(verbose=true)` and `getmempooldescendants(verbose=true)` are deprecated and will be removed in the next major version (use `-deprecated=fees` if needed in this version). The same fee fields can be accessed through the `fees` object in the result.

    **WARNING: deprecated fields `ancestorfees` and `descendantfees` are denominated in sats, whereas all fields in the `fees` object are denominated in DASH.**

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 578be36
  PastaPastaPasta:
    utACK 578be36

Tree-SHA512: 38a62ec826f91c349269373463dd47f662cdc14dbb3c82cb5c56e779f22a2623fdce055018f15bcf6b5da029312dbcce0925545650c0eae8e3f5177812b68321
ab4cus pushed a commit to ab4cus/e4Coin-core that referenced this pull request Jul 8, 2026
…ivated long time ago

751f414 docs: added a comment about removed SCRIPT_ENABLE_DIP0020_OPCODES (Konstantin Akimov)
4c051e0 feat: drop SCRIPT_ENABLE_DIP0020_OPCODES, make opcodes available from genesis block (Konstantin Akimov)
df3331c feat: remove feature_dip0020_activationl.py functional test and related code (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  it's alternate solution for dashpay/dash#6299 to fix a crash reported dashpay/dash#6256

  ## What was done?
  Removed code related to DIP0020 activation for various OP codes. DIP0020 is activated long time ago and no any historical blocks are violating rules, removing it's backwards compatible.

  ## How Has This Been Tested?
  Run unit and functional tests. See also changes in data for unit tests and removed functional test.
  It also re-index mainnet and testnet successfully
  ```
  src/qt/dash-qt  -reindex -assumevalid=0
  src/qt/dash-qt -testnet -reindex -assumevalid=0
  ```

  Also extra test is done with bitcoin/bitcoin#23590 - no crash with it in `feature_dip0020_activation.py` [modified assuming it is always activated]

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 751f414
  PastaPastaPasta:
    utACK 751f414

Tree-SHA512: 05ddda4e8fb66305995e91c8a04fbda690aef8fb82acb23b7d62f302da60b5ec7e7a97bd988efd2523dbd9cafde9f4b65cae2db9e4b5257464ce1c8fcca6a40f
ab4cus pushed a commit to ab4cus/e4Coin-core that referenced this pull request Jul 8, 2026
…24310, #24537, #24582, #24404, #25013, #25029, #25254, #25388 (mempool backports)

58c0c86 merge bitcoin#25388: move policy constants to policy (Kittywhiskers Van Gogh)
29bb5bd merge bitcoin#25254: Move minRelayTxFee to policy/settings (Kittywhiskers Van Gogh)
976f1e8 merge bitcoin#25029: Move fee estimation RPCs to separate file (Kittywhiskers Van Gogh)
68bbe47 merge bitcoin#25013: Remove cs_main from verifymessage, move msg utils to new file (Kittywhiskers Van Gogh)
78126cf merge bitcoin#24404: Remove confusing P1008R1 violation in ATMPArgs (Kittywhiskers Van Gogh)
0f4308d merge bitcoin#24582: Move txoutproof RPCs to txoutproof.cpp (Kittywhiskers Van Gogh)
d48141d merge bitcoin#24537: Split mempool RPCs from blockchain.cpp (Kittywhiskers Van Gogh)
b676f5c merge bitcoin#24310: docs / fixups from RBF and packages (Kittywhiskers Van Gogh)
4a51009 merge bitcoin#23804: followups for de-duplication of packages (Kittywhiskers Van Gogh)
11f7496 merge bitcoin#22674: mempool validation and submission for packages of 1 child + parents (Kittywhiskers Van Gogh)
3983c70 merge bitcoin#23381: refactoring for package submission (Kittywhiskers Van Gogh)
bc21cdd merge bitcoin#22689: deprecate top-level fee fields in getmempool RPCs (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * [bitcoin#23694](bitcoin/bitcoin#23694) (backported in [dash#6256](dashpay/dash#6256)) was backported before [bitcoin#22689](bitcoin/bitcoin#22689) and therefore, the formatting changes in the former were not incorporated in the backport. This has been resolved by accommodating them in the latter.

  * The fields deprecated in [bitcoin#22689](bitcoin/bitcoin#22689) were originally deprecated in [bitcoin#12240](bitcoin/bitcoin#12240) but not gated behind runtime arguments, the backport does so.

  * This pull request includes multiple backports that include code specific to RBF (BIP 125) and SegWit, which have been removed due to Dash Core not supporting them, comments and documentation (like `doc/policy/packages.md`) have been amended to reflect this accordingly.

  * When backporting [bitcoin#22674](bitcoin/bitcoin#22674), a new constructor was introduced for submitting `MEMPOOL_ENTRY` `ResultType`, upstream this can be done trivially as it uses function overloading ([source](https://github.com/bitcoin/bitcoin/pull/22674/files#diff-d3c243938494b10666b44404a27af7d84b44a72b85a27431e0c89e181462ca6eR198-R204)) but as Dash Core doesn't accept
  `replaced_txns`, there is no opportunity to differentiate arguments.
    * This was resolved by accepting `ResultType` as an argument but always passing `MEMPOOL_ENTRY` as the argument ([source](dashpay/dash@11f7496#diff-d3c243938494b10666b44404a27af7d84b44a72b85a27431e0c89e181462ca6eR232-R242))

  * Unit tests like `package_witness_swap_tests` (introduced in [bitcoin#23804](bitcoin/bitcoin#23804)) have been omitted due to functionality tested not being implemented in Dash Core. Subsequent backports that include modifications to these tests have mirrored similar omissions.

  ## Breaking Changes

  * The top-level fee fields `fee`, `modifiedfee`, `ancestorfees` and `descendantfees` returned by RPCs `getmempoolentry`,`getrawmempool(verbose=true)`, `getmempoolancestors(verbose=true)` and `getmempooldescendants(verbose=true)` are deprecated and will be removed in the next major version (use `-deprecated=fees` if needed in this version). The same fee fields can be accessed through the `fees` object in the result.

    **WARNING: deprecated fields `ancestorfees` and `descendantfees` are denominated in sats, whereas all fields in the `fees` object are denominated in DASH.**

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 58c0c86
  PastaPastaPasta:
    utACK 58c0c86

Tree-SHA512: 38a62ec826f91c349269373463dd47f662cdc14dbb3c82cb5c56e779f22a2623fdce055018f15bcf6b5da029312dbcce0925545650c0eae8e3f5177812b68321
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants