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

GH-16 Implement mempool #240

Merged
merged 1 commit into from
Oct 3, 2017
Merged

GH-16 Implement mempool #240

merged 1 commit into from
Oct 3, 2017

Conversation

lucafavatella
Copy link
Contributor

@lucafavatella lucafavatella commented Sep 29, 2017

Closes GH-16.
Complies to GH-236.
Includes API for updating the mempool when receiving new transactions (GH-72).

Supersedes PR #238.

Using iterator for not creating the list of all txs in memory from
storage using `gb_trees:iterator/1` was initially [drafted by
Artur](f4319ba#diff-2e95a69d7723aebad303814cb3f38e7eR92),
though passing the iterator to the calling process (hence moving it
across process heaps).

Sorting integers in decreasing order by using opposite of fee was
[drafted by
Artur](f4319ba#diff-2e95a69d7723aebad303814cb3f38e7eR99).

Wrt key, apart from fee in key for sorting, the rest of the key can be
made smaller. E.g. [Artur had
drafted](c375b04#diff-2e95a69d7723aebad303814cb3f38e7eR100)
using a key resulting from concatenation of opposite of fee and some
hash of the transaction:

```
Hash = aec_tx:hash(SignedTx),
Key = <<MinusFee:64,Hash/binary>>,
```
@lucafavatella lucafavatella self-assigned this Sep 29, 2017
@lucafavatella lucafavatella changed the title [WIP] GH-16 Implement mempool GH-16 Implement mempool Sep 29, 2017
@lucafavatella lucafavatella removed their assignment Sep 29, 2017
Copy link
Contributor

@zp-sd zp-sd left a comment

Choose a reason for hiding this comment

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

Looks good. One question added.

?assertEqual({ok, [STx4]}, aec_tx_pool:peek(10)),

%% A tx already in chain now received from peers.
?assertEqual(ok, aec_tx_pool:push(STx2)),
Copy link
Contributor

@zp-sd zp-sd Oct 3, 2017

Choose a reason for hiding this comment

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

If STx2 is already in a chain and we re-add it to the pool, such tx will be removed from the pool in mining attempt? Mining shall recognize it as duplicate then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If STx2 is already in a chain and we re-add it to the pool, such tx will be removed from the pool in mining attempt?

Txs are removed from pool when added in chain - see GH-236. Mining itself is not meant to remove txs from pool. After successful mining, if mined block added to chain then txs in block shall be then removed.

Mining shall recognize it as duplicate then?

Yes, thanks to nonce. Mining attempts to apply txs. Each tx is meant to contain initiating account and its nonce, and each tx is meant to ensure nonce in account is strictly less than nonce in txs. So duplicated tx is detected by mining, and mining shall delete such tx from pool.

Copy link
Contributor

@sennui sennui left a comment

Choose a reason for hiding this comment

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

Luca, I can approve it as the first iteration on the mempool. I think we should improve the API (and internal db) so it lets remove confirmed transactions in batch from the pool.

@sennui sennui merged commit f16dfce into master Oct 3, 2017
@sennui sennui mentioned this pull request Oct 3, 2017
7 tasks
@lucafavatella lucafavatella deleted the gh-16-mempool branch October 5, 2017 09:16
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.

None yet

3 participants