Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

DevGuide: Add Short Subsection About The Memory Pool #449

Merged
merged 3 commits into from Jun 13, 2014

Conversation

Projects
None yet
3 participants
Contributor

harding commented Jun 11, 2014

The memory pool gets discussed quite often, but my research didn't turn up anything particularly interesting about it, so this ended up being a short subsection.

Contributor

saivann commented Jun 11, 2014

Thanks! This text is clear and easy to understand. Maybe the people who are interested might point out exactly what they think is worth mentioning.

In case this creates some inspiration, perhaps it would be interesting to mention what happens to the mempool when there's a block chain reorganization (short and long forks).

Contributor

harding commented Jun 11, 2014

@saivann describing the mempool during a reorg is an excellent idea! I'll research exactly what happens and add that to this PR. Thanks!

@luke-jr luke-jr and 2 others commented on an outdated diff Jun 11, 2014

_includes/guide_p2p_network.md
{% endautocrossref %}
+#### Memory Pool
+
+{% autocrossref %}
+
+Full peers keep track of unconfirmed transactions which are eligible to
+be included in the next block. This is essential for miners who will
@luke-jr

luke-jr Jun 11, 2014

Contributor

Non-miners don't have to do this to be a full node.

@saivann

saivann Jun 11, 2014

Contributor

@luke-jr s/Full peers keep track of.../Full peers usually relay and keep track of... ?

@luke-jr

luke-jr Jun 11, 2014

Contributor

Sure

@harding

harding Jun 11, 2014

Contributor

@saivann @luke-jr I was thinking we take @luke-jr comment literally ("don't have to") and just add "may" to the sentence: Full peers may...

@saivann

saivann Jun 11, 2014

Contributor

@harding To my understanding, this would be fine too.

@luke-jr luke-jr commented on an outdated diff Jun 11, 2014

_includes/guide_p2p_network.md
{% endautocrossref %}
+#### Memory Pool
+
+{% autocrossref %}
+
+Full peers keep track of unconfirmed transactions which are eligible to
+be included in the next block. This is essential for miners who will
+actually mine some or all of those transactions, but it's also useful
+for any peer who wants to keep track of unconfirmed transactions, such
+as the receiver of a payment.
@luke-jr

luke-jr Jun 11, 2014

Contributor

Actually, it's not really useful for this. At least with Bitcoin Core, the wallet keeps track of your own transactions, ignoring the memory pool.

@luke-jr luke-jr commented on an outdated diff Jun 11, 2014

_includes/guide_p2p_network.md
+#### Memory Pool
+
+{% autocrossref %}
+
+Full peers keep track of unconfirmed transactions which are eligible to
+be included in the next block. This is essential for miners who will
+actually mine some or all of those transactions, but it's also useful
+for any peer who wants to keep track of unconfirmed transactions, such
+as the receiver of a payment.
+
+Because unconfirmed transactions have no permanent status in Bitcoin,
+Bitcoin Core stores them in non-persistent memory, calling them a memory
+pool or mempool. When a peer shuts down, its memory pool is lost except
+for any transactions stored by its wallet. This means that never-mined
+unconfirmed transactions tend to slowly disappear from the network as
+peers restart.
@luke-jr

luke-jr Jun 11, 2014

Contributor

There are also limits on sensible memory pool implementations, to prevent DoS by memory exhaustion.

@luke-jr luke-jr commented on an outdated diff Jun 11, 2014

_includes/guide_p2p_network.md
+be included in the next block. This is essential for miners who will
+actually mine some or all of those transactions, but it's also useful
+for any peer who wants to keep track of unconfirmed transactions, such
+as the receiver of a payment.
+
+Because unconfirmed transactions have no permanent status in Bitcoin,
+Bitcoin Core stores them in non-persistent memory, calling them a memory
+pool or mempool. When a peer shuts down, its memory pool is lost except
+for any transactions stored by its wallet. This means that never-mined
+unconfirmed transactions tend to slowly disappear from the network as
+peers restart.
+
+Transactions which a peer will not relay or mine may still be added to
+its memory pool, possibly in modified form. This can allow the peer to
+refuse double spends or child transactions, although this behavior is not
+used in Bitcoin Core as of this writing.
@luke-jr

luke-jr Jun 11, 2014

Contributor

It's also a potential attack vector against miners, since someone could fill their memory pool with transactions they won't mine, evicting ones they will.

Corrections To Memory Pool Text
Based on feedback from @luke-jr.  (Thanks!)

* Use "may" to indicate that full peers don't need to track unconfirmed txes.

* Change the example given for why non-mining peers might want to track
  unconfirmed txes.

* Note that txes also get dropped from mempool when memory needs
  to be freed.

* Remove paragraph about storing non-relay/non-mine txes.
Contributor

harding commented Jun 11, 2014

@luke-jr commit 847084b addresses your feedback. (Thanks!) Summary:

  • Use "may" to indicate that full peers don't need to track unconfirmed txes.
  • Change the example given for why non-mining peers might want to track unconfirmed txes.
  • Note that txes also get dropped from mempool when memory needs to be freed.
  • Remove paragraph about storing non-relay/non-mine txes.
Contributor

harding commented Jun 12, 2014

@saivann per your suggestion, I added commit ad33a91 describing what happens during a reorg. This is based on DisconnectTip() in Bitcoin Core main.cpp and related code. Thanks!

Contributor

saivann commented Jun 12, 2014

@harding Thanks! LGTM

Contributor

harding commented Jun 12, 2014

@saivann thanks!

If no critical feedback is received, I will merge this sometime after 04:00 UTC Friday.

@harding harding merged commit ad33a91 into bitcoin-dot-org:master Jun 13, 2014

@harding harding deleted the harding:memorypool branch Feb 25, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment