Skip to content

WIP: Mempool specification#1056

Merged
ebuchman merged 3 commits intodevelopfrom
feature/mempool-spec
Jan 21, 2018
Merged

WIP: Mempool specification#1056
ebuchman merged 3 commits intodevelopfrom
feature/mempool-spec

Conversation

@ethanfrey
Copy link
Contributor

Writing a public specification of the mempool module.

  • Include overview and configuration options.

Include overview and configuration options.
@ethanfrey ethanfrey force-pushed the feature/mempool-spec branch from b308b3a to 7b52499 Compare January 4, 2018 16:11

`--mempool.recheck=false` (default: true)

`--mempool.recheck_empty=false` (default: true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ebuchman Do we want to make this false by default?

mempool.go:368 suggests it is used for debugging

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe. But currently the mempool is supposed to be configured so a correct proposer will only propose valid txs. Apps that use EndBlock could invalidate transactions in the mempool without the mempool knowing, and so a correct proposer could return invalid txs.

We just need to be clear either way. There's def a performance benefit to setting it false (ie. we won't replay all our mempool txs when an empty block is committed ...)

@codecov-io
Copy link

codecov-io commented Jan 4, 2018

Codecov Report

Merging #1056 into develop will decrease coverage by 0.22%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1056      +/-   ##
===========================================
- Coverage    59.95%   59.72%   -0.23%     
===========================================
  Files          116      116              
  Lines        10667    10667              
===========================================
- Hits          6395     6371      -24     
- Misses        3701     3721      +20     
- Partials       571      575       +4

@xla xla mentioned this pull request Jan 5, 2018
@@ -0,0 +1,59 @@
# Mempool Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

there is #792. let's try to avoid duplication. thanks

Copy link
Contributor

@melekes melekes Jan 8, 2018

Choose a reason for hiding this comment

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

@zramsay maybe you can incorporate some things below


Mempool exposes `CheckTx([]byte)` over the RPC interface.

It can be posted via `broadcast_commit`, `broadcast_sync` or
Copy link
Contributor

Choose a reason for hiding this comment

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

these are not the same things... broadcast_commit calls both CheckTx and waits for DeliverTx result. not sure this should be here. i think this belongs to RPC API!

@ebuchman
Copy link
Contributor

Merging, will clean up later

@ebuchman ebuchman merged commit 6679fef into develop Jan 21, 2018
@ebuchman ebuchman deleted the feature/mempool-spec branch January 21, 2018 17:39
@zramsay
Copy link
Contributor

zramsay commented Jan 21, 2018

do we want any of these rendered in RTD?

firelizzard18 pushed a commit to AccumulateNetwork/tendermint that referenced this pull request Feb 1, 2024
* Update the annotation of part_set.go

* Update types/part_set.go

Co-authored-by: Sergio Mena <sergio@informal.systems>

---------

Co-authored-by: Mansub Song <60084364+mansub1029@users.noreply.github.com>
Co-authored-by: Sergio Mena <sergio@informal.systems>
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.

5 participants