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

Implement the mempool #16

Closed
joelreymont opened this issue Aug 28, 2017 · 10 comments
Closed

Implement the mempool #16

joelreymont opened this issue Aug 28, 2017 · 10 comments
Assignees

Comments

@joelreymont
Copy link
Contributor

joelreymont commented Aug 28, 2017

  1. Push transactions into the mempool, e.g. when received by a node.
  2. Pop transactions from the mempool, highest fee first.
  3. Delete transactions from the mempool when a block that includes these transactions is received.
@joelreymont joelreymont added the 2d label Aug 28, 2017
@joelreymont joelreymont modified the milestone: Sprint 1 Aug 28, 2017
@joelreymont joelreymont added this to the Sprint 1 milestone Sep 5, 2017
@joelreymont joelreymont changed the title Implement the mempool Implement the sync to mempool interface Sep 5, 2017
@joelreymont joelreymont changed the title Implement the sync to mempool interface Implement the mempool Sep 5, 2017
cytadela8 added a commit that referenced this issue Sep 6, 2017
@cytadela8
Copy link
Contributor

Problems that emerged when implementing mempool with sorted transactions:

  • No way to check if transaction is fully valid exposes us to getting invalid transactions and not realizing that.
    • How to penalize peer for giving bad transactions?
    • We need a process that will remove invalid transactions. Maybe we should add block creation to tx_pool and include such code in block_creation?
  • Need know fee for transaction solved by adding another wrapper like signed_tx

@joelreymont
Copy link
Contributor Author

Artur, please make sure that comments on a ticket can be understood without digging around
for information. Include links to the code that you are referring to, line numbers included.

Please elaborate on the following

  • Why is there no way to check if a transaction is fully valid? Please explain.
  • Why do we need to penalize the peer?
  • What does add block creation to tx_pool mean?
  • Why can't we ensure that invalid transactions do not make it into the mempool?
  • What do you mean by Need know fee for transaction solved by adding another wrapper like signed_tx.

@sennui

@cytadela8
Copy link
Contributor

Why is there no way to check if a transaction is fully valid? Please explain.

Why can't we ensure that invalid transactions do not make it into the mempool?

Transactions in one block may depend on each other. We sort transactions based on fees, previously we added transactions to the end of list. Previously we could just try to apply new transaction on the end state of trees after all the others. Now It's not possible, because new transaction should be applied between other transactions and other transaction may conflict with it.

Why do we need to penalize the peer?

It's a good idea to stop listening to someone that provides us with invalid transactions and makes us waste resources.

What does add block creation to tx_pool mean?

Maybe code for generating a block to mine should be included in tx_pool module. I'm thinking about that, because that process includes full transaction validation and we need to add some process to remove invalid transactions from the pool.

What do you mean by Need know fee for transaction solved by adding another wrapper like signed_tx.

That's an implementation detail. I added a wrapper like signed_tx to solve problem with accessing fee from mempool. I don't like my solution.

@sennui
Copy link
Contributor

sennui commented Sep 7, 2017

Lets walk it step by step:

Invalid transactions caused by fee-based re-ordering:
We should not worry about it. Fee based ordering is in favour of our customers and this is also powerful tool to prioritize transactions. They will figure out a way to order them so they are valid

invalid transactions
they are validated during adding to the next-gen tree. If one is invalid it's thrown out by miner
Sync with @zp-sd on how to communicate it between miner and mempool

penalizing the peer:
sure we can keep some counter - Doesn't look like MVP!

general:
Mind that mempool is kind of mining "manager". It interfaces with external world. It has several duties:

  • take new transaction and dispatch them to mining
  • take new block from external peer (consider it validated by Update state of the world when receiving blocks #71) and filter out already blocked transactions
  • be able to kill mining in case of above ^^
  • be able to dispatch transaction to mining with fee ordering

what is a good place to prevent replayed transactiones? Will they be caught by transaction validation (and nonces constraints?) @zp-sd @cytadela8

@zp-sd
Copy link
Contributor

zp-sd commented Sep 7, 2017

Another question: what should be done when a transaction is considered as invalid during mining (e.g. changes account's balance to negative value)?

Such transaction is skipped when selecting set of transactions for a block to get mined. But should it be kicked out of the mempool as well? And - if yes - shall this information be propagated to another peers? Or maybe we we need to add a kind of TTL for invalid transactions in the mempool?

@sennui
Copy link
Contributor

sennui commented Sep 7, 2017

@zp-sd the question. @cytadela8 what are your thoughts on that?

I would avoid complications at the beginning. Just throw it away, don't sync about it, don't set TTL.

@joelreymont
Copy link
Contributor Author

+1 for doing it the simple way.

@cytadela8
Copy link
Contributor

cytadela8 commented Sep 10, 2017

First, sorry for late comment, my notifications on Github are flooded. I have to look for a way to get only the important updates there so I won't miss such important discussions

My worries about transactions validity and that it may change when new transactions are put between others is not about problems with transactions not getting included or ordered as someone would like, but with potential invalid transactions spam. If we are not able to validate transactions that reach our mempool we leave ourselves vulnerable to a spam attack. Someone might create a lot of invalid transactions in such a way that we won't realize that they are invalid till we try to generate a block with them. This for example might cause as to run out of RAM or if we protect ourselves from that we just won't accept any new transactions.

@joelreymont
Copy link
Contributor Author

We can only validate that the transaction signature is valid before adding it to the mempool.

We cannot execute the transaction and thus check the balance until the transaction is picked out of the mempool.

The cannot distinguish between transaction spam, as described by @cytadela8, and a high volume of regular transactions.

We can try to throttle a peer that sends a high volume of transactions, though. Also, do remember that transactions incur fees and we'll deduct these fees when picking transactions out of the pool, regardless of whether the transactions can be executed or not.

@joelreymont joelreymont removed this from the Sprint 1: Getting started milestone Sep 11, 2017
@cytadela8
Copy link
Contributor

I see we understand each other. As for the last comment: Keep in mind the fee may not be possible to be deducted as they may not be enough balance left to deduct it. (e.g. one account creates a 1 000 000 of transactions from which only one can be executed other's fail due to balance=0)

@joelreymont joelreymont added this to the Sprint 3 milestone Sep 25, 2017
@lucafavatella lucafavatella self-assigned this Sep 25, 2017
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

No branches or pull requests

5 participants