Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CLI for TxId → BlockHash index #1340

Merged
merged 9 commits into from
Jul 1, 2021
Merged

CLI for TxId → BlockHash index #1340

merged 9 commits into from
Jul 1, 2021

Conversation

kfangw
Copy link
Contributor

@kfangw kfangw commented Jun 8, 2021 •
edited
Loading

closes #1316

add commands for indexing TxId->BlockHash

  • planet store build-index-tx-block [STORE] [OFFSET] [LIMIT]
  • planet store blocks-by-tx-id [STORE] [TX-ID]
  • planet store block-hashes-by-tx-id [STORE] [TX-ID]

@kfangw kfangw changed the title feat: CLI for TxId -> BlockHash index [WIP] feat: CLI for TxId -> BlockHash index Jun 8, 2021
@codecov
Copy link

codecov bot commented Jun 8, 2021 •
edited
Loading

Codecov Report

Merging #1340 (020fc77) into main (0033ed7) will decrease coverage by 8.95%.
The diff coverage is 53.70%.

❗ Current head 020fc77 differs from pull request most recent head 60e260c. Consider uploading reports for the commit 60e260c to get more accurate results

@@            Coverage Diff             @@
##             main    #1340      +/-   ##
==========================================
- Coverage   86.03%   77.08%   -8.96%     
==========================================
  Files         361      253     -108     
  Lines       32457    17070   -15387     
==========================================
- Hits        27925    13158   -14767     
- Misses       2747     3375     +628     
+ Partials     1785      537    -1248
Impacted Files Coverage Δ
...bplanet.Explorer/Controllers/ExplorerController.cs 0.00% <0.00%> (ø)
...rer/Controllers/GenericControllerNameConvention.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/Controllers/GraphQLBody.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/ExplorerStartup.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/GraphTypes/NodeStateType.cs 0.00% <0.00%> (ø)
...ibplanet.Explorer/Interfaces/IBlockChainContext.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/Queries/BlockQuery.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/Queries/ExplorerQuery.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/Queries/TransactionQuery.cs 0.00% <0.00%> (ø)
Libplanet.Explorer/Store/AddressRefDoc.cs 0.00% <0.00%> (ø)
... and 246 more

@kfangw kfangw self-assigned this Jun 9, 2021
@kfangw kfangw changed the title [WIP] feat: CLI for TxId -> BlockHash index feat: CLI for TxId -> BlockHash index Jun 14, 2021
@kfangw kfangw marked this pull request as ready for review June 14, 2021 07:26
@kfangw kfangw marked this pull request as draft June 17, 2021 00:46
@kfangw kfangw marked this pull request as ready for review June 17, 2021 01:09
@auto-assign auto-assign bot requested a review from limebell June 17, 2021 01:09
@limebell
Copy link
Member

Please resolve conflict in changelog.

@kfangw
Copy link
Contributor Author

kfangw commented Jun 28, 2021

PTAL.

@longfin longfin self-requested a review June 28, 2021 01:36
@kfangw
Copy link
Contributor Author

kfangw commented Jun 28, 2021

/rebase

limebell
limebell reviewed Jun 29, 2021
View reviewed changes
Libplanet.Extensions.Cocona.Tests/Commands/StoreCommandTest.cs
private readonly Transaction<Utils.DummyAction> _transaction1;
private readonly Transaction<Utils.DummyAction> _transaction2;
private readonly Transaction<Utils.DummyAction> _transaction3;
private readonly Transaction<Utils.DummyAction> _transaction4;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about making an array?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about the benefit of doing that.
Is there any weak point declaring 4 individual variables in this test case? @limebell

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just because it's clean? Not a strong suggestion though. Please check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, those variables don't share any semantics, such as living in the same block or signed by the same signer, in the following testcases. So I think that there is no need to aggregate them. And the beauty or cleanliness of code is up to the individuals unless it does not break the coding rule(lint)

Hence If you don't mind, may I keep them as a separate variable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🆗

@longfin longfin self-requested a review June 29, 2021 04:47
longfin
longfin previously approved these changes Jun 29, 2021
View reviewed changes
@kfangw kfangw added this to the 0.12 milestone Jul 1, 2021
@dahlia dahlia changed the title feat: CLI for TxId -> BlockHash index CLI for TxId → BlockHash index Jul 1, 2021
@kfangw
Copy link
Contributor Author

kfangw commented Jul 1, 2021

/rebase

@longfin longfin requested review from limebell and longfin July 1, 2021 06:43
longfin
longfin previously approved these changes Jul 1, 2021
View reviewed changes
limebell
limebell previously approved these changes Jul 1, 2021
View reviewed changes
@kfangw kfangw dismissed stale reviews from limebell and longfin via 60e260c July 1, 2021 07:18
@limebell limebell requested review from longfin and limebell July 1, 2021 08:17
@kfangw kfangw merged commit 77d8785 into planetarium:main Jul 1, 2021
@kfangw kfangw deleted the feat/txbindexcli branch July 1, 2021 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@limebell limebell limebell approved these changes

@dahlia dahlia Awaiting requested review from dahlia

@greymistcube greymistcube Awaiting requested review from greymistcube

@riemannulus riemannulus Awaiting requested review from riemannulus

@longfin longfin Awaiting requested review from longfin

Assignees

@kfangw kfangw

Labels
None yet
Projects
None yet
Milestone
0.12
Development

Successfully merging this pull request may close these issues.

Add planet store index command
4 participants
@kfangw @limebell @longfin @greymistcube