Skip to content

ABCI / mempool: Add a "Recheck Tx" function #2127

Closed
@ValarDragon

Description

@ValarDragon

In addition to CheckTx, we should also have a "Recheck Tx". The functionality would basically be the same, but it would be intended for use on things that have already passed a CheckTx once before. The reason for this is that the app could then skip certain checks, such as signature verification. Regardless of state changes, for most txs the signatures will remain valid. If there was a "MsgChangePubkey" this could be checked in the application logic, and in that case assoc. signature re-checked. (Given that this doesn't yet exist, doesn't seem like a concern right now)

Then we would call recheck in the mempool instead of check.

Right now rechecking every tx on each new block, and therefore doing each signature verification is going to be a significant hit on performance if you have a non-trivially filled mempool. Recall that our current Secp signature verification implementation is 505039 ns/op. Right now the bitcoin mempool is at 15MB. (https://www.blockchain.com/charts/mempool-size). The average bitcoin tx is ~300 bytes, which means that there are 52428 txs in the mempool. At signature verification time we have, this means a recheck would take 26 seconds. Thus the proposed scheme seems to me be a very important concern, as the mempool rechecking processing time between blocks would be 26 seconds. (Perhaps divided by a small constant factor due to parallelization)

Activity

added
C:abciComponent: Application Blockchain Interface
C:mempoolComponent: Mempool
T:perfType: Performance
on Aug 1, 2018
added this to the post-launch milestone on Aug 1, 2018
ValarDragon

ValarDragon commented on Aug 28, 2018

@ValarDragon
ContributorAuthor
ebuchman

ebuchman commented on Aug 30, 2018

@ebuchman
Contributor

Nice idea. Though technically this could be performed app side right now by keeping an in-memory map of txs we've successfully run CheckTx for, yeh? It would be like replicating the mempool cache in the app.

ValarDragon

ValarDragon commented on Aug 30, 2018

@ValarDragon
ContributorAuthor

Sure it could, but that breaks abstraction barriers. We want handling of sorting and priorities to belong in the mempool. (We don't want the app maintaining an additional map of all txs) That also seems more wasteful in terms of resources.

I feel like having a separate ABCI message (or alternatively additional boolean in CheckTx) is more optimal. I'm not opposed to adding a boolean in CheckTx to signal if were rechecking or not.

ebuchman

ebuchman commented on Aug 30, 2018

@ebuchman
Contributor

Right. Maybe the time has finally come for a RequestCheckTx type

ValarDragon

ValarDragon commented on Aug 30, 2018

@ValarDragon
ContributorAuthor

That sounds super useful to me!

17 remaining items

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

Metadata

Metadata

Assignees

Labels

C:abciComponent: Application Blockchain InterfaceC:mempoolComponent: MempoolT:enhancementType: EnhancementT:perfType: Performance

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions

    ABCI / mempool: Add a "Recheck Tx" function · Issue #2127 · tendermint/tendermint