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

A technique to access cc transactions in the connecting block #405

Open
dimxy opened this issue Mar 4, 2021 · 6 comments
Open

A technique to access cc transactions in the connecting block #405

dimxy opened this issue Mar 4, 2021 · 6 comments

Comments

@dimxy
Copy link
Collaborator

dimxy commented Mar 4, 2021

In cc modules a tx could spend another tx while it is staying in mempool, so when those two txns are added to the chain they are put in the same block.
So in the cc validation code we should provide that functions like myGetTransaction or CCgetspenttxid could access txns in the connecting block (when the cc validation code is run).
Currently this task is done as follows:
before a block is connected all mempool txns are removed and stored in a temp object, then mempool is filled with the block's transactions, the original mempool state is restored, the block is validated.
There are several issues with this approach:

  • we need to lock mempool for some time what affects performance
  • currently sometimes mempool cleanup is done inaccurately that creates situations when a tx is added in a block and stays in the mempool at the same time
  • if the mempool is mixed with the already stored txns and the block's txns, this might affect functions checking mempool in the validation code. So we must clean the mempool completely and fill it with only block txns during the block validation and therefore we need to lock mempool even for longer time.

There is another experimental implementation of this feature, in an additional repository (we worked out it with decker).
This implementation does not change the mempool state at all but instead has a special version of myGetTransaction (eval->GetTxUnconfirmed) for use inside validation which accesses block txns directly (instead of mempool) when the block is connected.
This solution does not change the mempool state so no performance issues and no need to cleanup. Also there is no side effects if the validation code checks if a tx is spent in mempool (as the mempool state might be different on different nodes).
What we need to do for this case is make special functions for use inside the validation code that will look into connecting block transactions instead of mempool.
(would like to get opinions, @ca333, @Alrighttt, @jl777, @DeckerSU)

@Alrighttt
Copy link
Member

Could you give specific examples of validation code that will use this? Please go into detail as to why GetTxUnconfirmed is insufficient.

@dimxy
Copy link
Collaborator Author

dimxy commented Mar 9, 2021

An example is any myGetTransaction call which refers to a previous transaction in the same block (that is, when added to mempool this tx was spending another tx sitting in mempool. Spending in mempool is enabled by default by myGetTransaction).
The function eval->GetTxUnconfirmed currently just redirects to myGetTransaction. I suggest to teach it to look into the connecting block as well (instead of simply redirecting to myGetTransaction).
Another candidate for this is CCgetspenttxid function, we also could make eval->CCgetspenttxid version for use in the validation code, which also will be looking into connecting block txns

@jmjatlanta
Copy link

jmjatlanta commented Nov 12, 2021

Below is a SWOT at what I see as the problem and a possible solution. I appreciate comments and corrections. This will be helpful in clarifying my understanding of the problem and "missing pieces" in my knowledge of the existing process.

Adjusting the mempool during transaction validation

Situation

The code validating transactions entering the mempool is very similar to validating transactions that are received in a block. A major difference is what the mempool contains when those processes complete.

Weapons

The existing code works and must keep working. This provides existing processes that provide a baseline (templates to work with).

Objectives

The existing code is overly complicated which makes adding features difficult and error-prone. New code that is performant, uncomplicated, and centralized (easy to adjust) should replace the existing sections of code.

Tactics

The mempool is not important to block addition until the end, but is important to adding transactions to the mempool. Therefore, transaction validation should be passed an object that is similar (has the same interface) to a mempool. I will call this a TransactionPool.

A TransactionPool is passed to the transaction validation code.

  • In the process of adding a transaction to the actual mempool, the TransactionPool is simply the global CTxMempool. If the validation passes, the global mempool is modified accordingly.
  • When processing a block, the TransactionPool is a collection of transactions within the block. If the validation passes and the block is added to the chain, any transactions within the TransactionPool are removed from the global mempool.

Such a strategy allows the validation code to receive minimal changes and work regardless of the context of why the validation code was called.

Methods such as myGetTransaction can look in the "pool" that is passed in as a parameter, and then look to the chain if necessary.

In the case of processing a block, "removing" a transaction from the pool because it was successfully validated has implications to the real mempool. Such transactions should be marked so that further transaction validation code sees the transaction as "available" or "spent" and successful block validation sees the transaction as needing to be removed from the true mempool if it exists.

@jmjatlanta
Copy link

jmjatlanta commented Nov 12, 2021

Status: I have been working on some unit tests in the jmj_testutils branch ( PR #453 ) to provide a baseline of existing functionality. They exist in the file src/test-komodo/test_block.cpp. I will branch from there once the details of the desired changes emerge.

Reference: how SmartUSD did it: https://github.com/pbcllc/sfusd-core/blob/b4ccf54db8e6a1dfa87f1756d273c1bb0cca463d/src/validation.cpp#L2249

@dimxy
Copy link
Collaborator Author

dimxy commented Nov 22, 2021

TransactionPool is a collection of transactions within the block

Seems TransactionPool is interesting idea (I suppose you mean it is an instance of CTxMempool). So we switch pools from the global to the TransactionPool when a block is connected. This would allow to use some services of the CtxMempool like check if an output is spent in the pool, maybe search functions.
From the other hand, we don't need all available functions like remove()
Still I think we should use 'wrapped' functions as members of Eval inside cc validation code. This would allow to provide needed context and also utilise the existing test framework for cc with mock data

@jmjatlanta
Copy link

jmjatlanta commented Nov 23, 2021

Seems TransactionPool is interesting idea (I suppose you mean it is an instance of CTxMempool).

Think of CTxMempool as a child class of the TransactionPool class.

So we switch pools from the global to the TransactionPool when a block is connected. This would allow to use some services of the CtxMempool like check if an output is spent in the pool, maybe search functions. From the other hand, we don't need all available functions like remove()

Agreed

Still I think we should use 'wrapped' functions as members of Eval inside cc validation code. This would allow to provide needed context and also utilise the existing test framework for cc with mock data

I will research the implications. Thanks again for your input.

who-biz pushed a commit to who-biz/komodo that referenced this issue Jun 14, 2022
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

No branches or pull requests

3 participants