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

pyropy · 2023-09-19T12:29:34Z

Continues upon #2897 . Closes #2836

pyropy · 2023-09-19T12:32:02Z

@gakonst here's a PR for the issue I've mentioned in #4282. Could we get issue #2836 assigned to me?

bin/reth/src/db/clear.rs

bin/reth/src/db/get.rs

onbjerg · 2023-09-26T00:09:30Z

crates/storage/db/src/tables/mod.rs

-    (SyncStage, TableType::Table),
-    (SyncStageProgress, TableType::Table),
-    (PruneCheckpoints, TableType::Table)
+    (CanonicalHeaders, TableType::Table, view),


guess this is ok, wish we didn't have to though

Me too, I was trying to find a better way to refactor the macro. I am unhappy with this way but I lack ideas. I'm up for suggestions tho

pyropy · 2023-09-26T08:19:25Z

@onbjerg How could I test this? I am new to the project so I am unfamiliar with the workflows.

dragan2234 · 2023-09-26T22:55:35Z

@pyropy find a table with duplicated keys and try to get the value with the subkey with your newly created command. I don't know if this is enough for testing(if you should write automated tests - that's the team decision). Before this command, db get table_key was returning the first subkey value in dupsort tables, now it should return exact if you pass the subkey too

codecov · 2023-09-27T14:43:07Z

Codecov Report

Attention: 67 lines in your changes are missing coverage. Please review.

Comparison is base (4dc15c3) 26.07% compared to head (8e91e9d) 67.95%.
Report is 460 commits behind head on main.

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

Additional details and impacted files

Files	Coverage Δ
crates/storage/db/src/tables/mod.rs	`53.65% <0.00%> (+24.39%)`	⬆️
bin/reth/src/db/clear.rs	`5.88% <0.00%> (+5.88%)`	⬆️
bin/reth/src/utils.rs	`2.08% <0.00%> (+0.01%)`	⬆️
bin/reth/src/db/get.rs	`68.60% <0.00%> (+68.60%)`	⬆️
bin/reth/src/db/list.rs	`3.96% <0.00%> (+3.96%)`	⬆️

... and 428 files with indirect coverage changes

Flag	Coverage Δ
integration-tests	`17.06% <0.00%> (-9.01%)`	⬇️
unit-tests	`62.10% <0.00%> (?)`

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

Components	Coverage Δ
reth binary	`30.60% <0.00%> (+4.82%)`	⬆️
blockchain tree	`80.82% <ø> (+52.36%)`	⬆️
pipeline	`88.36% <ø> (+83.32%)`	⬆️
storage (db)	`74.60% <0.00%> (+44.63%)`	⬆️
trie	`94.96% <ø> (+72.43%)`	⬆️
txpool	`55.33% <ø> (+13.95%)`	⬆️
networking	`78.17% <ø> (+47.28%)`	⬆️
rpc	`57.80% <ø> (+31.32%)`	⬆️
consensus	`63.01% <ø> (+37.94%)`	⬆️
revm	`24.47% <ø> (+14.62%)`	⬆️
payload builder	`7.95% <ø> (-6.21%)`	⬇️
primitives	`84.71% <ø> (+55.53%)`	⬆️

rkrasiuk · 2023-10-12T13:25:47Z

@pyropy you can simply initialize a holesky node and query for the storage slots of the deposit contract. do you have availability to bring this PR to completion?

pyropy · 2023-10-16T18:46:55Z

@pyropy you can simply initialize a holesky node and query for the storage slots of the deposit contract. do you have availability to bring this PR to completion?

Hey yes I'll do it over the weekend, I am in on-site camp with my company so I cannot do it ATM.

rkrasiuk · 2023-10-19T07:14:58Z

@pyropy fyi, the dupsort tables were wrongfully disabled on db list #5068

pyropy · 2023-10-23T06:07:51Z

@rkrasiuk I've synced holesky network and tried out the cli, here's the example for PlainStorageState table:

./target/debug/reth db get PlainStorageState 0x4242424242424242424242424242424242424242 0x0000000000000000000000000000000000000000000000000000000000000004 --chain=holesky
{
  "key": "0x0000000000000000000000000000000000000000000000000000000000000004",
  "value": "0x156663e7936e7dd38052dbffe5c8a41e507ccef69867efd1af269df0d0c7a8fe"
}

rkrasiuk · 2023-10-23T07:02:53Z

@pyropy sg! my remaining concern is the need for view_dupsort function. do we still need to differentiate given #5068?

pyropy · 2023-10-23T07:07:49Z

@pyropy sg! my remaining concern is the need for view_dupsort function. do we still need to differentiate given #5068?

Good call, let me take a look at that today and refactor my PR if needed.

gakonst · 2023-11-13T19:53:25Z

Hi @pyropy wonder if there's any next steps for this PR? Thanks for taking the time!

pyropy · 2023-11-15T14:05:25Z

Hi @pyropy wonder if there's any next steps for this PR? Thanks for taking the time!

Hey I think it should be good. If you can give it a review you I'd appreciate it 🙏🏻

bin/reth/src/db/tui.rs

bin/reth/src/utils.rs

bin/reth/src/db/list.rs

bin/reth/src/utils.rs

rkrasiuk · 2023-12-21T07:53:01Z

@pyropy thanks for the PR, we'll be merging it shortly. refactored the tables macro to a bit more amicable format

pyropy · 2023-12-21T09:12:01Z

@pyropy thanks for the PR, we'll be merging it shortly. refactored the tables macro to a bit more amicable format

Seen your refactor, good job. This is a really good way for me to learn how to do rust properly as I mainly write golang.

DaniPopes

LGTM. Fmt nits. IMO the two table macros should be merged in a follow-up.

bin/reth/src/db/get.rs

blocker

pyropy requested review from rakita, joshieDo and onbjerg as code owners September 19, 2023 12:29

pyropy changed the title ~~Feat/cli duptable db get~~ Feature: Support db get for duptables (CLI) Sep 19, 2023

onbjerg previously requested changes Sep 26, 2023

View reviewed changes

pyropy requested a review from onbjerg September 26, 2023 08:18

rkrasiuk added C-enhancement New feature or request A-cli Related to the reth CLI labels Oct 12, 2023

rkrasiuk self-requested a review October 12, 2023 13:26

Dragan Pilipovic and others added 3 commits October 22, 2023 11:59

work in progress

421109b

Pass TableViewer view fn into macro

b4c1bf9

Remove redundant copy calls

36ff44d

pyropy force-pushed the feat/cli_duptable_db_get branch 2 times, most recently from c60fea2 to 1c53ade Compare October 22, 2023 10:04

Use DupSort bound instead Table + DupSort

8e91e9d

pyropy force-pushed the feat/cli_duptable_db_get branch from 1c53ade to 8e91e9d Compare October 22, 2023 10:25

pyropy added 2 commits November 15, 2023 14:38

Merge branch 'main' into feat/cli_duptable_db_get

9cfdcc7

Add raw arg to list

8ce381b

github-actions bot added S-stale This issue/PR is stale and will close with no further activity and removed S-stale This issue/PR is stale and will close with no further activity labels Dec 7, 2023

rkrasiuk added 2 commits December 18, 2023 20:18

refactor tables macro to treat table & dupsort separately

fe7fbab

Merge branch 'main' into feat/cli_duptable_db_get

fb241b0

rkrasiuk reviewed Dec 18, 2023

View reviewed changes

rkrasiuk added 4 commits December 18, 2023 10:39

Apply suggestions from code review

555bb87

remove unwraps

5eb68a4

fix from str impl

b676abc

fix fmt

bb183e9

rkrasiuk added 2 commits December 21, 2023 09:17

fix test and docs

d1dfa05

delegate to view by default

731be03

DaniPopes approved these changes Dec 21, 2023

View reviewed changes

bin/reth/src/db/get.rs Outdated Show resolved Hide resolved

bin/reth/src/db/get.rs Outdated Show resolved Hide resolved

bin/reth/src/db/get.rs Outdated Show resolved Hide resolved

fmt

12d25fd

rkrasiuk enabled auto-merge December 21, 2023 14:36

rkrasiuk added this pull request to the merge queue Dec 21, 2023

Merged via the queue into paradigmxyz:main with commit b37cd83 Dec 21, 2023
27 checks passed

pyropy deleted the feat/cli_duptable_db_get branch February 27, 2024 21:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

pyropy commented Sep 19, 2023

pyropy commented Sep 19, 2023

onbjerg Sep 26, 2023

pyropy Sep 26, 2023

pyropy commented Sep 26, 2023

dragan2234 commented Sep 26, 2023 •

edited

Loading

codecov bot commented Sep 27, 2023 •

edited

Loading

rkrasiuk commented Oct 12, 2023

pyropy commented Oct 16, 2023

rkrasiuk commented Oct 19, 2023

pyropy commented Oct 23, 2023 •

edited

Loading

rkrasiuk commented Oct 23, 2023

pyropy commented Oct 23, 2023

gakonst commented Nov 13, 2023

pyropy commented Nov 15, 2023

rkrasiuk commented Dec 21, 2023

pyropy commented Dec 21, 2023

DaniPopes left a comment

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

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

Conversation

pyropy commented Sep 19, 2023

pyropy commented Sep 19, 2023

onbjerg Sep 26, 2023

Choose a reason for hiding this comment

pyropy Sep 26, 2023

Choose a reason for hiding this comment

pyropy commented Sep 26, 2023

dragan2234 commented Sep 26, 2023 • edited Loading

codecov bot commented Sep 27, 2023 • edited Loading

Codecov Report

rkrasiuk commented Oct 12, 2023

pyropy commented Oct 16, 2023

rkrasiuk commented Oct 19, 2023

pyropy commented Oct 23, 2023 • edited Loading

rkrasiuk commented Oct 23, 2023

pyropy commented Oct 23, 2023

gakonst commented Nov 13, 2023

pyropy commented Nov 15, 2023

rkrasiuk commented Dec 21, 2023

pyropy commented Dec 21, 2023

DaniPopes left a comment

Choose a reason for hiding this comment

dragan2234 commented Sep 26, 2023 •

edited

Loading

codecov bot commented Sep 27, 2023 •

edited

Loading

pyropy commented Oct 23, 2023 •

edited

Loading