stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

Feat/mempool admission logic

Open pavitthrap opened this issue 3 years ago • 4 comments

Description

Addresses #2774. With this change, any transaction posted to the v2/transactions will be accepted into the mempool if it is able to be validated against either the unconfirmed state or the confirmed state.

In addition, this PR adds the optional query parameter mempool_admission_check to the v2/transactions endpoint. If mempool_admission_check=="unconfirmed", the transaction will only be validated against unconfirmed state (and then accepted into the mempool if that validation succeeds). If mempool_admission_check=="confirmed", the transaction will only be validated against confirmed state (and then accepted into the mempool if that validation succeeds).

Note that if the unconfirmed state is uninitialized, then specifying mempool_admission_check=="unconfirmed" will validate the transaction against the previous anchored tip.

Type of Change

  • [x] New feature
  • [ ] Bug fix
  • [ ] API reference/documentation update
  • [ ] Other

Are documentation updates required?

Added docs to openapi.yaml

Testing information

Added test to neon_integrations.rs

pavitthrap avatar Jul 29 '21 16:07 pavitthrap

This LGTM, @pavitthrap, but I am a little concerned that the Any case can end up doing 2x admission checks for an invalid tx. Would it make sense for the Any case to just check against the unconfirmed state (if it exists)? Basically, eliminate the Any case and make the default OffChainOnly.

Sure, just implemented this. So I decided to leave the type of mempool_admission_check as the TransactionAnchorMode enum. I wanted to keep it as an enum (as opposed to a bool) since I thought that was clearer.

pavitthrap avatar Aug 17 '21 18:08 pavitthrap

Ooops -- I spoke too soon. It looks like the failing integration test revealed an interesting consequence of the change: it appears that the mempool admission failed because it could not find the unconfirmed MARF trie. I think this error scenario should be explored before merging.

Right -- I was afraid of this. The code needs to verify that the underlying MARF trie actually exists before proceeding to use the unconfirmed chain tip.

jcnelson avatar Aug 19 '21 00:08 jcnelson

@pavitthrap I'm curious about where this PR stands these days and whether a release ETA might be available?

markmhendrickson avatar Sep 16 '21 10:09 markmhendrickson

@pavitthrap I'm curious about where this PR stands these days and whether a release ETA might be available?

Update here: https://github.com/blockstack/stacks-blockchain/issues/2774#issuecomment-922077240

pavitthrap avatar Sep 17 '21 20:09 pavitthrap

This will be unblocked soon with @pavitthrap's work on the microblock anti-entropy protocol.

jcnelson avatar Feb 07 '23 16:02 jcnelson

Stale, closing this PR

saralab avatar Oct 27 '23 00:10 saralab