Skip to content

Conversation

@furszy furszy self-assigned this Jun 25, 2020
@furszy furszy force-pushed the 2020_6915 branch 2 times, most recently from 2c7a1d3 to b3496d3 Compare June 25, 2020 03:48
@furszy furszy changed the title [WIP] RPC & Mempool back ports. RPC & Mempool back ports. Jun 25, 2020
@random-zebra
Copy link

Would be good to have bitcoin/bitcoin@fa1436c and bitcoin/bitcoin@fac2fc4 fully backported here (i.e. not only sync_blocks(), but also sync_mempools())

@furszy
Copy link
Author

furszy commented Jun 25, 2020

@random-zebra done.
We still need to back port syncwithvalidationinterfacequeue RPC command in a future PR.

@furszy furszy requested review from Fuzzbawls and random-zebra June 28, 2020 23:31
Reserve key is only used in PoW phase, no need to have it for PoS.
@furszy
Copy link
Author

furszy commented Jul 2, 2020

conflicts solved. Rebased on top of master.

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Looking good. Just few things.

  • It would be good to add the sync_mempool assert to 3895a6115bfd03cf4f4642ea5b19ae67f422f62b. And (minor thing) f98c964f75fb336240e4c01a8d60466564219093 is still missing the changes to mempool_reorg test from bitcoin/bitcoin@fac2fc4

  • Currently there is a rpc_fundrawtransaction.py failure due to rounding error after the subtraction. To fix it, just round the tx output value to 8 decimals in test_locked_wallet:

    outputs = {self.nodes[0].getnewaddress(): round(250.0 - float(self.test_no_change_fee) - float(self.fee_tolerance), 8)}
  • Found an error in miner_tests.cpp, not spotted by the compiler, as it seems we are not including that test in the Make file yet.

furszy and others added 11 commits July 3, 2020 17:43
coming from btc@9b060e5cfb0d185b553b21ae19d390f81e83bd4d
coming from btc@b0a064c4b825c15dee87739348bab23f13541bdd
Coming from btc@bb8ea1f6304d7ed3f5fe0a01c060ac9f94629349
Coming from btc@71f1d9fd4ae2c2fc90d5487bdf2096f9eb5898d9
Coming from btc@5945819717eb842df28cd9291a226d0505cb49d0
Compute the value of inputs that already are in the chain at time of mempool entry and only increase priority due to aging for those inputs.  This effectively changes the CTxMemPoolEntry's GetPriority calculation from an upper bound to a lower bound.

Coming from btc@c0353064ddf71ad103bd19f6e7c10ff8e240ac46
Coming from btc@7e49f5f8b4e237d7212d027a7bea4bbd52c9e7b6
Add a function `ParseFixedPoint` that parses numbers according
to the JSON number specification and returns a 64-bit integer.

Coming from btc@fed500e2dd571406b9420f7a26a5db6dee801806
furszy and others added 7 commits July 3, 2020 18:43
coming from btc@598b25d5ee9c08947a52824f47531208943a3c65
CalculateMemPoolAncestors was always looping over a transaction's inputs
to find in-mempool parents.  When adding a new transaction, this is the
correct behavior, but when removing a transaction, we want to use the
ancestor set that would be calculated by walking mapLinks (which should
in general be the same set, except during a reorg when the mempool is
in an inconsistent state, and the mapLinks-based calculation would be the
correct one).
Coming from btc@fa1436c42958688bcc48418f5b434a80a53fd5ac
Coming from btc@fac3716b09bb9ee121db629873d9694a95cae942
…nt failure

Coming from btc@fac2fc4dd8a28b99e17c57e4ab6580a3231f1d0a
@furszy
Copy link
Author

furszy commented Jul 3, 2020

Solved all the comments. Ready for another round of review.

Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

Overall,
ACK 830eadf

minor non-blocking nit about direct header inclusion that can be addressed in a followup PR.


// Each thread has its own key and counter
CReserveKey reservekey(pwallet);
Optional<CReserveKey> opReservekey{nullopt};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: should probably include our optional.h wrapper directly in the header includes rather than implicitly

Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK 830eadf and merging...

@random-zebra random-zebra merged commit 6bc917e into PIVX-Project:master Jul 6, 2020
random-zebra added a commit that referenced this pull request Aug 18, 2020
6c566bb Add TxPriority class and comparator (Alex Morcos)
5e9df89 Make accessing mempool parents and children public (Alex Morcos)
1279a9f Add a score index to the mempool (furszy)
e5a72fe Store the total sig op count of a tx (furszy)

Pull request description:

  On top of #1707 and #1717 .

  Back ports bitcoin#6898 (without back porting the last commit as we need some cleanup on the miner area first).

ACKs for top commit:
  random-zebra:
    ACK 6c566bb
  Fuzzbawls:
    utACK 6c566bb

Tree-SHA512: c6fa788c59ea6e024e340e926e81462d0817617003034eecf9c3eeda3dfd5fbd2a7b6ec1ada3b867dba98079a195931f8a72fc2250207f475d8f44060452825b
@furszy furszy deleted the 2020_6915 branch November 29, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants