Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Legitimately map transactions to statuses in blocktree #7011

Merged

Conversation

CriesofCarrots
Copy link
Contributor

Problem

getConfirmedBlock RPC returns dummy transaction status data, because Blocktree::map_transactions_to_statuses() isn't hooked up.

Summary of Changes

  • Get data from TransactionStatus db column and map to transactions by Signature
  • Refactor TransactionStatus store and RpcConfirmedBlock to use a named struct for clarity. RpcConfirmedBlock wraps this RpcTransactionStatus in an Option in order to allow getConfirmedBlock to function on validators that have not enabled the TransactionStatus store. (Statuses that cannot be found return None.) When Implement getConfirmedBlock RPC method #6867 item 5 is implemented, it should probably also prevent map_transactions_to_statuses() from firing at all to save all the db get attempts on an empty column.

... Since there are no apis writing data to the TransactionsStatus store, getConfirmedBlock returns None for all RpcTransactionStatuses.

Toward #6867

This will break getConfirmedBlock in https://github.com/solana-labs/solana-web3.js again, again

@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

Merging #7011 into master will decrease coverage by 13.8%.
The diff coverage is 87.3%.

@@            Coverage Diff            @@
##           master   #7011      +/-   ##
=========================================
- Coverage    74.4%   60.6%   -13.9%     
=========================================
  Files         223     223              
  Lines       45495   55952   +10457     
=========================================
+ Hits        33883   33937      +54     
- Misses      11612   22015   +10403

@sunnygleason
Copy link
Contributor

(nobody asked but lgtm 😄 )

@CriesofCarrots CriesofCarrots merged commit e0a2bb9 into solana-labs:master Nov 18, 2019
@CriesofCarrots CriesofCarrots deleted the blocktree-tx-to-status branch February 21, 2020 22:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants