Repository: MystenLabs/sui
File: crates/sui-rpc-api/src/grpc/v2/ledger_service/get_transaction.rs (Rust)

Code before:
                    .with_reason(ErrorReason::FieldInvalid)
            })?;
        FieldMaskTree::from(read_mask)
    };

    let transaction_read = service.reader.get_transaction_read(transaction_digest)?;

    if requires_indexed_data(&read_mask) {
        let highest_indexed = service
            .reader
            .inner()
            .indexes()
            .and_then(|idx| idx.get_highest_indexed_checkpoint_seq_number().ok())
            .flatten()
            .unwrap_or(0);

        if transaction_read
            .checkpoint
            .is_none_or(|cp| cp > highest_indexed)
        {
            return Err(RpcError::new(
                tonic::Code::NotFound,
                "Transaction is not yet indexed",
            ));
        }
    }

    let transaction = transaction_to_response(service, transaction_read, &read_mask)?;

    Ok(GetTransactionResponse::new(transaction))
}

#[tracing::instrument(skip(service))]
pub fn batch_get_transactions(
    service: &RpcService,
    BatchGetTransactionsRequest {
        digests, read_mask, ..
    }: BatchGetTransactionsRequest,
) -> Result<BatchGetTransactionsResponse, RpcError> {
    let read_mask = {
        let read_mask = read_mask.unwrap_or_else(|| FieldMask::from_str(READ_MASK_DEFAULT));
        read_mask
            .validate::<ExecutedTransaction>()
            .map_err(|path| {
                FieldViolation::new("read_mask")
                    .with_description(format!("invalid read_mask path: {path}"))
                    .with_reason(ErrorReason::FieldInvalid)
            })?;
        FieldMaskTree::from(read_mask)
    };


Review comment (suggestion):
I don't think this does exactly what we want, especially in the case where indexes are disabled and a user gives a wildcard * read mask.

I think instead what we'd want is to do something slightly different and do the following so that we only return information about a transaction IFF the fullnode has actually executed the checkpoint containing the transaction:

- Use `ReadStore::get_transaction_checkpoint` to look up the checkpoint for a transaction (this column family is stored in the checkpoint/perpetual store and not the rpc indexes so it'll always return `Some(checkpoint)` for transactions that exist on the node. 
- If you get `None` from that call then we can just bail with a 404
- next read the latest executed checkpoint `ReadStore::get_latest_checkpoint` and compare that sequence number to the transaction's checkpoint number and make sure it is >= otherwise return a 404
- If we get here then we can fill out the response as normal

Code after:
                    .with_description(format!("invalid read_mask path: {path}"))
                    .with_reason(ErrorReason::FieldInvalid)
            })?;
        FieldMaskTree::from(read_mask)
    };

    let Some(transaction_checkpoint) = service
        .reader
        .inner()
        .get_transaction_checkpoint(&transaction_digest.into())
    else {
        return Err(TransactionNotFoundError(transaction_digest).into());
    };

    let latest_checkpoint = service
        .reader
        .inner()
        .get_latest_checkpoint()?
        .sequence_number;

    if transaction_checkpoint > latest_checkpoint {
        return Err(TransactionNotFoundError(transaction_digest).into());
    }

    let transaction_read = service.reader.get_transaction_read(transaction_digest)?;

    let transaction = transaction_to_response(service, transaction_read, &read_mask);

    Ok(GetTransactionResponse::new(transaction))
}

#[tracing::instrument(skip(service))]
pub fn batch_get_transactions(
    service: &RpcService,
    BatchGetTransactionsRequest {
        digests, read_mask, ..
    }: BatchGetTransactionsRequest,
) -> Result<BatchGetTransactionsResponse, RpcError> {
    let read_mask = {
        let read_mask = read_mask.unwrap_or_else(|| FieldMask::from_str(READ_MASK_DEFAULT));
        read_mask
            .validate::<ExecutedTransaction>()
            .map_err(|path| {
                FieldViolation::new("read_mask")
                    .with_description(format!("invalid read_mask path: {path}"))
                    .with_reason(ErrorReason::FieldInvalid)
            })?;
        FieldMaskTree::from(read_mask)
    };

    let latest_checkpoint = service