rpc: Fix short-circuit condition going into big-table#12068
Conversation
#### Problem While updating the bigtable fallback logic in solana-labs#7937, we incorrectly short-circuited the check into bigtable. Currently, we always go into bigtable if `until` is not specified, even if we've hit the limit of transactions to return. #### Summary of changes Only respect `found_until` if `until` is provided.
|
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
|
If this PR represents a change to the public RPC API:
Thank you for keeping the RPC clients in sync with the server API @joncinque. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #12068 +/- ##
=========================================
- Coverage 83.2% 83.2% -0.1%
=========================================
Files 861 861
Lines 322768 322768
=========================================
- Hits 268743 268740 -3
- Misses 54025 54028 +3 🚀 New features to boost your workflow:
|
steviez
left a comment
There was a problem hiding this comment.
Makes sense and confirmed that found_until is false if the until sig is None:
agave/ledger/src/blockstore.rs
Lines 4002 to 4003 in 81e58b2
The change looks fine to fix + BP by keeping it minimal. Afterwards, I'm thinking we might consider using the type system to avoid the ambiguity / the bug. Namely, make this an Option<bool>:
agave/ledger/src/blockstore.rs
Line 125 in 81e58b2
Then we'd have:
- None =>
untilwasn't set so we didn't look in bstore / don't need to look in bigtable - Some(false) =>
untilwas set but we didn't find it in bstore so go check bigtable - Some(true) =>
untilwas set and we found it in bstore so skip bigtable
|
Going to merge on your behalf to get CI rolling on the BP |
… of #12068) (#12167) #### Problem While updating the bigtable fallback logic in #7937, we incorrectly short-circuited the check into bigtable. Currently, we always go into bigtable if `until` is not specified, even if we've hit the limit of transactions to return. #### Summary of changes Only respect `found_until` if `until` is provided. (cherry picked from commit 1a36fe0) Co-authored-by: Jon C <me@jonc.dev>
Problem
While updating the bigtable fallback logic in #7937, we incorrectly short-circuited the check into bigtable. Currently, we always go into bigtable if
untilis not specified, even if we've hit the limit of transactions to return.Summary of changes
Only respect
found_untilifuntilis provided.