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

Use applyChainTick before transaction validation #1291

Closed
edsko opened this issue Nov 25, 2019 · 2 comments · Fixed by #1295
Closed

Use applyChainTick before transaction validation #1291

edsko opened this issue Nov 25, 2019 · 2 comments · Fixed by #1295
Assignees
Labels
consensus issues related to ouroboros-consensus mempool
Milestone

Comments

@edsko
Copy link
Contributor

edsko commented Nov 25, 2019

The mempool must be able to validate transactions which have not yet been included in a block. At the moment, it validates these transactions against the current ledger state, in the assumption that they will be included in the "next" block. The set of transactions in the mempool is then updated every time a block comes in, and reevaluated against the new current ledger state.

This is not correct. The transactions in the mempool will be part of the body of a block, but a block consists of a header and a body, and the full validation of a block insists of first processing its header and only then processing the body. This is important, because processing the header may change the state of the ledger: the update system might be updated, scheduled delegations might be applied, etc., and such changes should take affect before we validate any transactions.

We therefore need to make sure, consensus side, that the transactions in the mempool are validated against the current ledger state to which we have applied the "chain tick" function for the header of the block in which they will end up. Of course, we don't have that header yet; it turns out, all we need is a slot, but we don't even have that slot number yet. We will therefore have to make some assumptions in the mempool: when we are validating transactions without producing a block (that is, when adding transactions into the mempool), we could assume the slot number is the next available one; when we are actually producing a block, then at that point we do have the correct slot number and we should use it.

This will also require some changes to the Byron wrapper. At the moment, the chain tick function only updates the update proposal state; it does not change either cvsLastSlot or the delegation state; for the validation to be correct, it should change both. Technically this requires changes to the spec (see IntersectMBO/cardano-ledger#1046), but since updates to the delegation state are idempotent, it's okay to (at least for the time being) do this consensus side (it will happen twice when we apply a block: once in the tick function and then again when ledger processes the block body, but that's not a big concern).

@edsko
Copy link
Contributor Author

edsko commented Nov 25, 2019

Unfortunately applyChainTick was only recently removed from the API (#1226); we will have to bring it back.

edsko added a commit that referenced this issue Nov 26, 2019
This undoes the changes from #1226. See #1291 and (new) docstring for
`applyChainTick` for motivation.

Note that I've changed the contract of `applyBlock` slightly: it is
respoinsible for processing the _entire_ block, including any chain
ticking; the chain tick function is needed only if we want do only
_part_ of the block processing.
@edsko
Copy link
Contributor Author

edsko commented Nov 26, 2019

#1293 reintroduces the chain tick function and extends the Byron chain tick function.

edsko added a commit that referenced this issue Nov 26, 2019
This undoes the changes from #1226. See #1291 and (new) docstring for
`applyChainTick` for motivation.

Note that I've changed the contract of `applyBlock` slightly: it is
respoinsible for processing the _entire_ block, including any chain
ticking; the chain tick function is needed only if we want do only
_part_ of the block processing.

It also extends the Byron chain tick function; see detailed discussion
in the comments.
iohk-bors bot added a commit that referenced this issue Nov 26, 2019
1293: Return of the slot tick r=edsko a=edsko

This is required before we can implement #1291 (which will then hopefully resolve a lot of concerns about the application of the ledger rules for transaction validation).

Co-authored-by: Edsko de Vries <edsko@well-typed.com>
edsko added a commit that referenced this issue Nov 26, 2019
edsko added a commit that referenced this issue Nov 26, 2019
iohk-bors bot added a commit that referenced this issue Nov 26, 2019
1295: Use applyChainTick in mempool r=edsko a=edsko

This closes #1291.

Co-authored-by: Edsko de Vries <edsko@well-typed.com>
@iohk-bors iohk-bors bot closed this as completed in 0afdd00 Nov 26, 2019
@tatyanavych tatyanavych reopened this Nov 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus mempool
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants