Feat/tx tracking #5484
Feat/tx tracking #5484
Conversation
| @@ -665,6 +680,23 @@ impl<'a> ChainstateTx<'a> { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| pub fn get_index_block_hashes_from_txid( | |||
There was a problem hiding this comment.
The only way this would return multiple rows is if a transaction was mined in two different blocks. Is that a query this function is intended to serve?
There was a problem hiding this comment.
Well the same txid can ends multiple times in the transactions table, obviously we are interested in txid that actually ends in the chain so it will be a 2 step process where we return only the row with a valid (in the staging db) index_block_hash. If a user knows the index_block_hash this would not be required, but at the same time if the index-block_hash is known there is no need for this endpoint (you can just extract the transaction from the block)
There was a problem hiding this comment.
Well the same txid can ends multiple times in the transactions table,
Can you explain to me how this happens?
|
Updated to draft to a true pull request, i have cleaned up the code and added a new HttpNotImplemented error (501) for when the transaction log is not enabled |
| .take() | ||
| .ok_or(NetError::SendError("`txid` no set".into()))?; | ||
|
|
||
| node.with_node_state(|_network, _sortdb, chainstate, _mempool, _rpc_args| { |
There was a problem hiding this comment.
It seems like you are trying to answer two separate queries here:
- What is the transaction for the given txid?
- Is the transaction on the canonical Stacks fork?
Answering the latter can be very, very expensive with the approach you have taken. You're effectively scanning an unbounded number of blocks' transactions, and doing MARF reads on each block relative to the chain tip.
I don't know if there is value in only returning transactions if they are on the canonical chain, since nearly all the time they will be. Instead, I think it's just as effective to return the transaction and the list of block IDs in which the node knows it occurs (which will almost always be one block). It would then be on the user to verify that those blocks are canonical.
There was a problem hiding this comment.
I asked in another thread how it could be possible for a transaction to be mined in multiple different Nakamoto blocks, and I think the only way this could happen is if there is a deep Nakamoto fork. This would only happen if there was a Bitcoin fork, and the second sibling Bitcoin block was mined substantially later than the first sibling, and the second (late) sibling became canonical. Nakamoto would preserve both Bitcoin forks and both Stacks forks atop them, and would write a transaction log entry for the same transaction in both forks.
It's not impossible, but it would be so unlikely that it might not be worth solving at this time.
There was a problem hiding this comment.
i like this approach way more than the current one, it will make the code way easier and extracting the tx from a block given the index_block_hash is straightforward.
I still get dozens of duplicated txid (with different index_block_hash) in my testnet transaction_log, i will try to understand why this is happening
| ### Changed | ||
|
|
||
| - Use the same burn view loader in both block validation and block processing | ||
| >>>>>>> develop |
|
Converting back to draft as i am changing the configuration logic as discussed during the nakamoto meetings |
Description
This PR implements another step of #4358, particularly the transactions query system.
As discussed previously this is an optional endpoint useful for implementers of services/abstraction apis that can be enabled with the already available STACKS_TRANSACTION_LOG environment variable.
The current transaction log system is based on storing each transaction in the "transactions" table of the index db. Each row contains the txid, the index_block_hash and the hex encoded body of the transaction. It means in the current form it requires a lot of storage and lot of transactions are basically orphaned ones.
The patch covers the first issue related to "testability", as the TRANSACTION_LOG value holding the status of the STACKS_TRANSACTION_LOG env var is a static per-process one. When in test mode TRANSACTION_LOG become a thread_local (instead of lazy_static) with the idea of each test requiring it calls borrow_mut() for setting it.
There is a bit of refactoring in the TRANSACTION_LOG check as now it is overlayed by an is_transaction_log_enabled() function. Should not be an issue as this will be inlined when in non-test mode.
It is not necessarily "the best" solution but only a surgical way for not thrashing the current code.
It is important that every test using it restores the original condition, so i have used a Drop-trait structure for keeping the state (i do not know how much rust-idiomatic it is honestly).
The user queries by txid, so one of the main task of the endpoint is to check for the txid with a valid index_block_hash (optionally by enforcing a specific tip qith the tip querystring)
Applicable issues
Additional info (benefits, drawbacks, caveats)
Currently the body of the transaction is extracted from the nakamoto staging db instead of the transactions table. This is definitely inefficient but i did it with the idea that probably we could remove the tx body itself from the transactions table (drastically reducing the size required for it, especially because i am really doubtful it will be useful to query invalid/orphaned transactions and the query of the mempool is already exposed by the /unconfirmed endpoint)
As an additional point, we could even reduce the impact of the patch by just returning the index_block_hash in the endpoint output instead of the whole body of the transaction (the tx body extraction can happen with an additional /v3/blocks query). Right now a json with the index_block_hash and the body is returned.
Checklist
docs/rpc/openapi.yamlandrpc-endpoints.mdfor v2 endpoints,event-dispatcher.mdfor new events)clarity-benchmarkingrepobitcoin-tests.yml