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

[Bug][indexer-grpc] Fullnode indexer grpc service ignores transactions_count in GetTransactionsFromNodeRequest #15628

Open
byron1st opened this issue Dec 18, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@byron1st
Copy link

🐛 Bug

The Indexer GRPC service of the fullnode seems to be ignoring the transactions_count field in the GetTransactionsFromNodeRequest. When the aptos-indexer-processor is built and run for the first time, it connects to the fullnode to retrieve the chain_id information by sending a GetTransactionsFromNodeRequest as shown below.

{
  "starting_version": 1,
  "transactions_count": 2,
}

However, the fullnode ignores the transactions_count field and instead processes transactions based on the processor_batch_size value specified in the configuration file. As a result, even though only 2 transactions were requested, it appears to return a response with 1000 transactions.

Looking at the related code here: fullnode_data_service.rs#L43, there seems to be no reference to transactions_count being used.

Could this be intended behavior, or is it possibly a minor oversight?

Or it could be that I don’t fully understand the default behavior of gRPC in Rust.

To reproduce

By enabling the Indexer settings on the fullnode and running it with the log level set to debug, you can verify this behavior through the fullnode logs when the default processor of the aptos-indexer-processor is executed during its initial startup.

Expected Behavior

If 2 transactions are requested, only 2 should be read from the database and processed.

System information

Please complete the following information:

  • Aptos Core Version: v1.20.1 and v1.21.2
  • Rust Version: 1.78.0
  • Computer OS: Ubuntu

Additional context

@byron1st byron1st added the bug Something isn't working label Dec 18, 2024
@rtso
Copy link
Contributor

rtso commented Dec 19, 2024

@byron1st To get a range of transactions, you should be using starting_version and ending_version. As you pointed out, transactions_count is an unused variable (cc @larry-aptos @grao1991 on looking into removing it so it doesn't confuse users).

@byron1st
Copy link
Author

Based on my observation of the code in

pub struct GetTransactionsFromNodeRequest {
, it seems that the GetTransactionsFromNodeRequest type does not have an ending_version field and only includes transactions_count. As a result, in the Aptos Indexer Processors code (https://github.com/aptos-labs/aptos-indexer-processors/blob/0cad20631225ee00313d40feaa916ef3a3f5a08c/rust/processor/src/grpc_stream.rs#L171), when making a GetTransactionsFromNode request, the ending_version is used to calculate the transactions_count, which is then included in the GetTransactionsFromNodeRequest.

However, as I pointed out, the transactions_count field in GetTransactionsFromNodeRequest appears to be ignored, and GetTransactionsFromNode RPC seems to always return 1,000 transactions, regardless of the input transactions_count.

Is there perhaps a different RPC definition that includes an ending_version field that I might be unaware of?

@rtso
Copy link
Contributor

rtso commented Dec 20, 2024

My bad, that's correct that GetTransactionFromNodeRequest accepts transactions_count as a parameter.

I just tested it using the following grpcurl command, which correctly returns 1 transaction:

docker run fullstorydev/grpcurl:v1.8.7 -d '{ "starting_version": 1496685738, "transactions_count": 1 }' -max-msg-sz 30000000 -H "authorization:Bearer {AUTH_KEY}" grpc.testnet.aptoslabs.com:443 aptos.indexer.v1.RawData/GetTransactions

Do you have an example where it's returning the wrong # of transactions?

@byron1st
Copy link
Author

If the correct number of transactions are being returned in the currently deployed testnet/mainnet, that is good. I am operating a private blockchain network consisting of several Aptos validators and fullnodes (using the images uploaded to Dockerhub without any code modifications).

Recently, during an update from v1.19.2 to v1.20.1, I encountered an issue where the Validator would panic whenever the Indexer (using the default processor from aptos-indexer-processor and built from the aptos-indexer-processor repository) connected to port 50051. Additionally, I observed through the logs that, even though the Indexer was set to fetch transactions_count of 2, the Validator attempted to read and process transactions from version 1 to version 1000.

The logs I reviewed pertained to info-level logs about transactions being read from Aptos’s local database. Therefore, I am not certain if 1,000 transactions were actually returned or if the system simply attempted to read 1,000 transactions but returned only the transactions_count specified. However, after reviewing the logs, I immediately checked the related code and found that the transactions_count parameter was not being used in the implementation. The relevant code is located at this link.

If this is working as intended, I may have overlooked something during my code analysis. Nevertheless, I wanted to report my observations for further investigation. If no issues are observed in the deployed version, you may close this issue. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants
@byron1st @grao1991 @rtso and others