Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mempool memory usage underutilized #8609

Closed
rebroad opened this issue Aug 27, 2016 · 10 comments
Closed

Mempool memory usage underutilized #8609

rebroad opened this issue Aug 27, 2016 · 10 comments

Comments

@rebroad
Copy link
Contributor

rebroad commented Aug 27, 2016

screenshot at 2016-08-27 10 07 27

In this example the mempool memory usage went to pretty much zero before eventually becoming useful again.

NOTE: This symptom becomes less pronounced when maxmempool and dbcache are increased.

UPDATE: The problem seems to occur just as much on another node with maxmempool and dbcache doubled - just at different times, so it may not be related to these values.

@rebroad rebroad changed the title MinRelayFee falls too slowly Mempool memory usage underutilized Aug 27, 2016
@gmaxwell
Copy link
Contributor

gmaxwell commented Sep 9, 2016

What reason do you have to believe that this isn't perfectly correct?

@laanwj
Copy link
Member

laanwj commented Sep 9, 2016

I don't get it. What is the problem here? The mempool size is a maximum cap, it willuse less than that after e.g. a block comes in and transactions are removed from it.

@sipa
Copy link
Member

sipa commented Sep 9, 2016 via email

@laanwj laanwj closed this as completed Sep 9, 2016
@rebroad
Copy link
Contributor Author

rebroad commented Sep 25, 2016

@laanwj you ask a question, and then close the issue the same day without giving me a chance to answer. Are issues only issues as long as you personally understand them?

Look at the graph half-way through - there is still a minimum relay fee even though the memory utilization is almost zero. This is the issue - why limit the txs at all when the utilization is almost 0%? Would it not make more sense to continue storing txs and only increase the restrictive criteria once utilization gets close to being 100%?

@sipa
Copy link
Member

sipa commented Sep 25, 2016

@rebroad One design goal of the limited mempool was to prevent relay of
transactions that don't pay for relay.

Consider the following scenario: a peer figures out what the lowest feerate
of any transaction in your mempool is (call it tx A), and creates a valid
transaction with a feerate 1sat/kB than this (call it tx B). If our mempool
is at its maximum size, and there was no minimum feerate at all, we would
kick out tx A and accept tx B, effectively replacing it. If this were to
happen across the network (for example because there are several nodes with
the same mempool size and software), the network as a whole would have
relayed tx A without it actually ending up in a block - it effectively
relayed across the network for free.

This could be exploited by anyone who observes the network wide behaviour,
and produces a series of transactions with very very slowly increasing fee
only, which at every node in the network cause the previous transaction to
be replaced by the newer one, for effectively ~zero cost. At a large scale,
this means someone could use the P2P network to mass relay data everywhere,
wasting bandwidth and processing time, without paying fees as a deterrent.

To combat this:

  • There always is a minimum feerate that we demand transactions pay
    (1000/kB currently, far lower than the going network rate, and the same as
    many versions ago).
  • When a transaction is being kicked out in favor of another transaction
    (whether through RBF or mempool limiting), we always require the feerate of
    the new one to pay at least the minimum feerate more than the kicked-out
    one paid. This guarantees that the latest transaction in a chain of
    replacements always paid for the relay (at minimum feerate level) of all
    previous evicted transactions before it.
  • After kicking out a transaction, we raise the effective minimum feerate
    temporarily to avoid using the newly available space (after a new block was
    mined) from being filled with relatively low-feerate transactions that
    necessitate being evicted again soon after anyway.

As you can see, for all of these countermeasures, we need an effective
minimum feerate even when the mempool is not full.

@laanwj
Copy link
Member

laanwj commented Sep 25, 2016

@laanwj you ask a question, and then close the issue the same day without giving me a chance to answer. Are issues only issues as long as you personally understand them?

Closing issues (in contrast to locking them) doesn't prevent anyone from answering.

To keep a lid on the sheer number of issues open, I have to do the thankless job of closing issues that are not clear user problems, but fall into other categories like "I've noticed this curious behavior and have questions about it". As I've said to you many times this is an issue tracker not a Q&A site.

This is also not the place to make other people do your research/investigation for you.

@rebroad
Copy link
Contributor Author

rebroad commented Oct 4, 2016

@laanwj I do not see github being used to do other people research/investigation, but I understand that you perceive this. I do not understand why though.

I think the root cause of this is ultimately lack of documentation. @sipa 's very detailed response, IMHO, ought to be incorporated into such documentation. There is the question of where this documentation ought to best be stored. My answer to that question would be github - with the documentation for the functionality of each pull request being stored against the pull request. Ideally the documentation ought to be submitted with the pull request itself, or in the code as comments, but when this isn't done, then at least, retrospectively might be a suitable alternative.

Perhaps I am unaware of such documentation that already exists - if so, then the issue, IMHO, would be making it easier for people to find it.

@maflcko
Copy link
Member

maflcko commented Oct 4, 2016

I think the root cause of this is ultimately lack of documentation.

I agree that there is a lack of documentation. Usually the code is considered the documentation and it becomes clear how something works, but the design principles usually are hidden somewhere else. (Maybe not even publicly accessible)

For the protocol and general bitcoin related concepts, we have the Bitcoin Improvement Proposals... For documenting internal logic, you suggested to use github issues and pull requests. I disagree, the design rationale for the internal logic should be placed in properly formatted doxygen comments above each class/method. (We have the problem, that those comments usually outdate very quickly because contributors "forget" to update the comments. Unfortunately I don't know of a way to enforce during review that comments need to be updated when the code is changed. Maybe only a maintainer for the specific module could keep it in mind? While not perfect, I still think that documentation should be part of the source code.)

@laanwj
Copy link
Member

laanwj commented Oct 4, 2016

While not perfect, I still think that documentation should be part of the source code.

100% agreed.

Also note that while it's easy to forget to update developer documentation that is part of the source code, it is even easier to forget to update "design documents" that live elsewhere. We actually have a few of those in doc/, for example https://github.com/bitcoin/bitcoin/blob/master/doc/multiwallet-qt.md .
Or the list of threads here: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads . Sufficient to say, both are outdated, though to different degrees.

Application user documentation should probably exist outside the source code, although it can exist as something that can be queried from the program itself. Like the RPC help, or --help flags. And e.g. a context-sensitive help for the GUI would be nice.

Protocol documentation should exist outside this repository as it is not specific to Bitcoin Core. Examples of this are the BIPs, as well as the excellent Developer Reference on bitcoin.org.

@rebroad
Copy link
Contributor Author

rebroad commented Dec 11, 2016

@sipa why would anyone want to relay txs around the network with no intention of them getting into blocks?

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants