feat: add duptable support for `reth db get` #2897

dragan2234 · 2023-05-29T14:23:29Z

dragan2234 · 2023-05-29T18:47:33Z

Tables:

    (TableType::DupSort, PlainStorageState::const_name()),
    (TableType::DupSort, AccountChangeSet::const_name()),
    (TableType::DupSort, StorageChangeSet::const_name()),
    (TableType::DupSort, HashedStorage::const_name()),
    (TableType::DupSort, StoragesTrie::const_name()),

dragan2234 · 2023-05-30T19:12:40Z

Right now it works this way:

reth db get-dup <table> <key> <subkey>

gakonst · 2023-05-31T19:45:10Z

bin/reth/src/db/get.rs

 }

+#[derive(Parser, Debug)]
+pub struct DupCommand {


We want to keep it under get, no need for a new command

gakonst · 2023-05-31T19:46:46Z

bin/reth/src/utils.rs

@@ -90,6 +91,11 @@ impl<'a, DB: Database> DbTool<'a, DB> {
        self.db.view(|tx| tx.get::<T>(key))?.map_err(|e| eyre::eyre!(e))
    }

+    /// Grabs the content of the table for the given key and subkey
+    pub fn get_dup<T: DupSort>(&mut self, key: T::Key, subkey: T::SubKey) -> Result<Option<T::Value>> {
+        self.db.view(|tx| tx.cursor_dup_read::<T>().unwrap().seek_by_key_subkey(key, subkey)).unwrap().map_err(|e| eyre::eyre!(e))


@rkrasiuk is this the right way? IIRC we cannot do this fully generically and need to use the structs we've defined for the subkeys by table?

pinging @rkrasiuk I'm back on the issue. seek_by_key_subkey and cursor_dup_read forces me to pass the Subkey here, so it's impossible having the same macro for both types of tables (Table and DupSort), even with new changes from #2935

So if this call with tx.cursor_dup_read::<T>().unwrap().seek_by_key_subkey(key, subkey) is the correct way of pulling data, we would need to change macro from https://github.com/paradigmxyz/reth/pull/2935/files#diff-bc5df8783f831a8e01372f13a49fbd1d2179d21dc309c8fdba3604bcd0dd372dR92 and create 2 separate macros for "normal" and "DupSort" tables. OR we create a method for DupSort tables but keep the view function only having fn view<T: Table> without DupSort.

Let me know your thoughts.

gakonst · 2023-05-31T19:47:36Z

bin/reth/src/db/get.rs

+        ]);
+    }
+
+    fn table_key<T: Table>(&self) -> Result<T::Key, eyre::Error>


This looks redundant if you combine the two commands

codecov-commenter · 2023-05-31T20:27:37Z

Codecov Report

Merging #2897 (8d45e47) into main (e8b09cc) will decrease coverage by 52.12%.
The diff coverage is 0.00%.

❗ Current head 8d45e47 differs from pull request most recent head ccb2f2b. Consider uploading reports for the commit ccb2f2b to get more accurate results

@@             Coverage Diff             @@
##             main    #2897       +/-   ##
===========================================
- Coverage   69.48%   17.36%   -52.12%     
===========================================
  Files         535      515       -20     
  Lines       71761    66778     -4983     
===========================================
- Hits        49860    11595    -38265     
- Misses      21901    55183    +33282

Flag	Coverage Δ
integration-tests	`17.36% <0.00%> (+1.01%)`	⬆️
unit-tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
bin/reth/src/db/get.rs	`0.00% <0.00%> (-75.65%)`	⬇️
bin/reth/src/db/mod.rs	`0.00% <0.00%> (-10.39%)`	⬇️
bin/reth/src/utils.rs	`0.00% <0.00%> (-4.69%)`	⬇️

... and 429 files with indirect coverage changes

dragan2234 · 2023-05-31T21:13:31Z

Thanks for the review. Just to note: there is this pr: #2935 which needs to be merged before merging this one since it changes a few things related to this pr

dragan2234 · 2023-06-25T21:06:40Z

what I'm looking at right now is how to generate the view_dupsort function inside the same macro, but only for DupSort table type. Since that function would need to have type view_dupsort. I'm looking into the macro's identifiers, and expressions but still struggling to do it correctly

dragan2234 changed the title ~~init work~~ WIP: Add duptable support for reth db get May 29, 2023

onbjerg changed the title ~~WIP: Add duptable support for reth db get~~ feat: add duptable support for reth db get May 29, 2023

onbjerg added C-enhancement New feature or request A-cli Related to the reth CLI labels May 29, 2023

dragan2234 marked this pull request as ready for review May 30, 2023 19:34

dragan2234 requested a review from onbjerg as a code owner May 30, 2023 19:34

gakonst requested changes May 31, 2023

View reviewed changes

onbjerg added the S-blocked This cannot more forward until something else changes label Jun 19, 2023

dragan2234 marked this pull request as draft June 23, 2023 00:46

work in progress

ccb2f2b

dragan2234 force-pushed the feat/cli_duptable_db_get branch from 95ed6d0 to ccb2f2b Compare June 23, 2023 00:51

github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Aug 1, 2023

github-actions bot closed this Aug 9, 2023

This was referenced Sep 12, 2023

Api call to display completeness of initial sync #4282

Open

Feature: Support db get for duptables (CLI) #4653

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add duptable support for `reth db get` #2897

feat: add duptable support for `reth db get` #2897

dragan2234 commented May 29, 2023 •

edited by onbjerg

Loading

dragan2234 commented May 29, 2023

dragan2234 commented May 30, 2023

gakonst May 31, 2023

gakonst May 31, 2023

dragan2234 Jun 22, 2023 •

edited

Loading

gakonst May 31, 2023

codecov-commenter commented May 31, 2023 •

edited by codecov bot

Loading

dragan2234 commented May 31, 2023

dragan2234 commented Jun 25, 2023

feat: add duptable support for reth db get #2897

feat: add duptable support for reth db get #2897

Conversation

dragan2234 commented May 29, 2023 • edited by onbjerg Loading

dragan2234 commented May 29, 2023

dragan2234 commented May 30, 2023

gakonst May 31, 2023

Choose a reason for hiding this comment

gakonst May 31, 2023

Choose a reason for hiding this comment

dragan2234 Jun 22, 2023 • edited Loading

Choose a reason for hiding this comment

gakonst May 31, 2023

Choose a reason for hiding this comment

codecov-commenter commented May 31, 2023 • edited by codecov bot Loading

Codecov Report

dragan2234 commented May 31, 2023

dragan2234 commented Jun 25, 2023

feat: add duptable support for `reth db get` #2897

feat: add duptable support for `reth db get` #2897

dragan2234 commented May 29, 2023 •

edited by onbjerg

Loading

dragan2234 Jun 22, 2023 •

edited

Loading

codecov-commenter commented May 31, 2023 •

edited by codecov bot

Loading