Make BlockchainProvider2 transaction id methods aware of in-memory state

[Rjected]

Right now these methods:

reth/crates/storage/provider/src/providers/blockchain_provider.rs

Lines 585 to 598 in a706206

	fn transaction_id(&self, tx_hash: TxHash) -> ProviderResult<Option<TxNumber>> {
	self.database.transaction_id(tx_hash)
	}
	fn transaction_by_id(&self, id: TxNumber) -> ProviderResult<Option<TransactionSigned>> {
	self.database.transaction_by_id(id)
	}
	fn transaction_by_id_no_hash(
	&self,
	id: TxNumber,
	) -> ProviderResult<Option<TransactionSignedNoHash>> {
	self.database.transaction_by_id_no_hash(id)
	}

just check the database for transactions. Previously this was fine because the database was always up to date with the canonical chain. Now, this is not the case, so these need to also check in-memory state.

This method also needs updating:

reth/crates/storage/provider/src/providers/blockchain_provider.rs

Lines 621 to 623 in a706206

	fn transaction_block(&self, id: TxNumber) -> ProviderResult<Option<BlockNumber>> {
	self.database.transaction_block(id)
	}

As do these methods:

reth/crates/storage/provider/src/providers/blockchain_provider.rs

Lines 679 to 695 in a706206

	fn transactions_by_tx_range(
	&self,
	range: impl RangeBounds<TxNumber>,
	) -> ProviderResult<Vec<TransactionSignedNoHash>> {
	self.database.transactions_by_tx_range(range)
	}
	fn senders_by_tx_range(
	&self,
	range: impl RangeBounds<TxNumber>,
	) -> ProviderResult<Vec<Address>> {
	self.database.senders_by_tx_range(range)
	}
	fn transaction_sender(&self, id: TxNumber) -> ProviderResult<Option<Address>> {
	self.database.transaction_sender(id)
	}

[mvares]

hey @Rjected, I'm so excited to see this PR being closed due to their complexity. I want to learn about how works the handling of queries over in-memory state.

[Rjected]

hey @Rjected, I'm so excited to see this PR being closed due to their complexity. I want to learn about how works the handling of queries over in-memory state.

hey! Just assigned you, let me know if you need any help or need a review!

[mvares]

Well, I did my homework. I thought and reach out in this solution:

1. Iterates over all in-memory blocks
2. Search in transactions by ID provided
3. Returns the block

reth/crates/storage/provider/src/providers/blockchain_provider.rs

Lines 621 to 623 in a706206

	fn transaction_block(&self, id: TxNumber) -> ProviderResult<Option<BlockNumber>> {
	self.database.transaction_block(id)
	}

cc @Rjected

[Rjected]

Well, I did my homework. I thought and reach out in this solution:

1. Iterates over all in-memory blocks

2. Search in transactions by ID provided

3. Returns the block

reth/crates/storage/provider/src/providers/blockchain_provider.rs

Lines 621 to 623 in a706206

	fn transaction_block(&self, id: TxNumber) -> ProviderResult<Option<BlockNumber>> {
	self.database.transaction_block(id)
	}

cc @Rjected

The fundamental problem with these txid methods, is that we don't really keep track of a txid index for in-memory blocks, unlike db blocks. So it would be good to do something like:

if let Some(num) = self.database.transaction_block(id)? {
    return Ok(Some(num))
} else {
    // what do we do here?
}

For the else branch, we should get the id for the latest block on disk, and iterate through in-memory blocks to determine which block the transaction is in. The id here is the tx number

[mvares]

Makes sense. Btw, why in-memory blocks don't works equal db blocks?

[Rjected]

Makes sense. Btw, why in-memory blocks don't works equal db blocks?

wdym? do you mean why do we not keep a txid index for in-memory blocks?

[mvares]

Makes sense. Btw, why in-memory blocks don't works equal db blocks?

wdym? do you mean why do we not keep a txid index for in-memory blocks?

yeah

[Rjected]

Makes sense. Btw, why in-memory blocks don't works equal db blocks?

wdym? do you mean why do we not keep a txid index for in-memory blocks?

yeah

ah, we just don't do it yet, since it is extra complexity for maintaining the in-memory state. If it seems like a bottleneck after measurement, then we may think about introducing an index

[mvares]

hey @Rjected, what do you think about if I unassign this? Sincerely, I still can't make queries like this due to my capacity

[mvares]

I will wait by solution. I want to see how will someone resolve it... Thanks for trust

[lakshya-sky]

Hey @Rjected, I gave it a try and I came up with the impl based on your comment #10181 (comment).

    fn transaction_block(&self, id: TxNumber) -> ProviderResult<Option<BlockNumber>> {
        if let Some(num) = self.database.transaction_block(id)? {
            return Ok(Some(num));
        }
        // Get First and Last block number from the in-memory state
        let last_block_number = self.last_block_number()?;
        let first_block_number = last_block_number.saturating_add(1);
        let canonical_block_number = self.canonical_in_memory_state.get_canonical_block_number();

        // Get next_tx_num from the last block stored in the database
        let Some(last_block_body_indice) = self.database.block_body_indices(last_block_number)?
        else {
            return Ok(None);
        };
        let mut next_tx_num = last_block_body_indice.next_tx_num();

        for number in first_block_number..=canonical_block_number {
            let Some(block_state) = self.canonical_in_memory_state.state_by_number(number) else {
                return Ok(None);
            };
            let txs_len = block_state.block().block().body.len();
            next_tx_num += txs_len as u64;
            if id < next_tx_num {
                return Ok(Some(number));
            }
        }
        Ok(None)
    }

I wanted to confirm with you that should I continue?