feat: introduce txid->blockhash index on IStore #1328

kfangw · 2021-06-03T05:24:32Z

APIs

Following APIs are added to IStore

void PutTxIdBlockHashIndex(TxId txId, BlockHash blockHash);
BlockHash? GetFirstTxIdBlockHashIndex(TxId txId);
IEnumerable<BlockHash> IterateTxIdBlockHashIndex(TxId txId);
void DeleteTxIdBlockHashIndex(TxId txId, BlockHash blockHash);

Contract

There exists only one record for a pair <TxId, BlockHash>.
There are multiple BlockHash paired with the same TxId (consider reorg situation.)

codecov · 2021-06-03T06:54:11Z

Codecov Report

Merging #1328 (0783fe8) into main (0033ed7) will decrease coverage by 9.10%.
The diff coverage is 53.04%.

❗ Current head 0783fe8 differs from pull request most recent head a15bf0d. Consider uploading reports for the commit a15bf0d to get more accurate results

@@            Coverage Diff             @@
##             main    #1328      +/-   ##
==========================================
- Coverage   86.03%   76.93%   -9.11%     
==========================================
  Files         361      253     -108     
  Lines       32457    16945   -15512     
==========================================
- Hits        27925    13036   -14889     
- Misses       2747     3380     +633     
+ Partials     1785      529    -1256

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 243 more

kfangw · 2021-06-03T08:42:05Z

/rebase

limebell

Please update this branch to main and resolve conflicts. 🙇‍♂️

CHANGES.md

Libplanet/Store/IStore.cs

Libplanet/Store/BaseStore.cs

longfin · 2021-06-05T06:42:39Z

/rebase

longfin · 2021-06-07T01:48:23Z

Libplanet.RocksDBStore/MonoRocksDBStore.cs

+        public override void PutTxIdBlockHashIndex(Guid chainId, TxId txId, BlockHash blockHash)
+        {
+            var cf = GetColumnFamily(_chainDb, chainId);
+            _chainDb.Put(


if we decide to store these indexes on chainDb, we also need to add a new method for forking because we can delete the previous chainDb after forking...

Or, how about using the separated DB for storing?

I was thinking that It(forking) is handled by other codebases.
And I realized that the chain id may not be needed. So the forking also not needed.
please see the patch 5f43072

Yes, if we don't put it with ColumnFamily, there is no matter. 😄

kfangw · 2021-06-07T03:11:31Z

if we decide to store these indexes on chainDb, we also need to add a new method for forking because we can delete the previous chainDb after forking...

Or, how about using the separated DB for storing?

The chain id may not be needed for this change. Please see the patch 5f43072

public void PutTxIdBlockHashIndex(TxId txId, BlockHash blockHash);
public bool HasTxIdBlockHashIndex(TxId txId);
public BlockHash? GetTxIdBlockHashIndex(TxId txId);
public void DeleteTxIdBlockHashIndex(TxId txId);

IMHO, TxId->BlockHash index can be handled like TxExecution.
What do you think about this? @longfin @dahlia

kfangw · 2021-06-08T08:38:36Z

The APIs were changed according to your advice @longfin

chain id is removed from the argument, and let the multiple pair <TxId, BlockHash> which share the same TxId.
Hence, IterateTxIdBlockHashIndex method is introduced to traverse the list of pairs by using the prefix(TxId).
And GetTxIdBlockHashIndex is changed to GetFirstTxIdBlockHashIndex method, which returns the first element or null if the list is empty.

void PutTxIdBlockHashIndex(TxId txId, BlockHash blockHash);
BlockHash? GetFirstTxIdBlockHashIndex(TxId txId);
IEnumerable<BlockHash> IterateTxIdBlockHashIndex(TxId txId);
void DeleteTxIdBlockHashIndex(TxId txId, BlockHash blockHash);

kfangw · 2021-06-08T08:43:14Z

Libplanet.RocksDBStore/RocksDBStore.cs

+        /// <inheritdoc cref="BaseStore.PutTxIdBlockHashIndex(TxId, BlockHash)"/>
+        public override void PutTxIdBlockHashIndex(TxId txId, BlockHash blockHash)
+        {
+            _txExecutionDb.Put(


Is it OK for you to share the same db with TxExecution? @dahlia
IMHO, txExecutionDB consists of transient and deterministic data. So they can share the same db. #1321

kfangw · 2021-06-09T02:21:07Z

PTAL

kfangw · 2021-06-14T00:27:23Z

PTAL. @longfin @dahlia @greymistcube @limebell @riemannulus
Can I merge it?

CI passed except for Codecov test coverage.
Codecov coverage degradation is caused by the exclusion of test projects.

longfin · 2021-06-14T01:18:22Z

Can you tell me when GetFirstTxIdBlockHashIndex() is useful? IMO, IterateTxIdBlockHashIndex() is sufficient in the general case.

longfin

Everything else looks fine.

kfangw · 2021-06-14T01:42:04Z

Can you tell me when GetFirstTxIdBlockHashIndex() is useful? IMO, IterateTxIdBlockHashIndex() is sufficient in the general case.

The API responds to the caller that a transaction is included in at least one block or not. It has a similar use case like hasXXX API.
As you mentioned, the caller can discover the fact thru the iterator API.
However, IMHO, this API may useful who want to know the fact without iterating them.

kfangw · 2021-06-14T01:45:56Z

/rebase

kfangw self-assigned this Jun 3, 2021

kfangw changed the title ~~[wip] feat: introduce txid->blockhash index on IStore~~ feat: introduce txid->blockhash index on IStore Jun 3, 2021

kfangw marked this pull request as ready for review June 3, 2021 08:42

auto-assign bot requested review from dahlia, greymistcube, limebell, longfin and riemannulus June 3, 2021 08:42

kfangw force-pushed the feat/txbindex branch from eb41d05 to 4e1848e Compare June 3, 2021 08:45

kfangw mentioned this pull request Jun 3, 2021

feat: indexing TxId->BlockHash when appending block #1329

Merged

limebell reviewed Jun 4, 2021

View reviewed changes

kfangw force-pushed the feat/txbindex branch from 4e1848e to 3cce7a1 Compare June 4, 2021 01:43

limebell self-requested a review June 4, 2021 01:54

dahlia reviewed Jun 4, 2021

View reviewed changes

kfangw force-pushed the feat/txbindex branch from c97189b to 5173022 Compare June 4, 2021 08:02

limebell previously approved these changes Jun 4, 2021

View reviewed changes

libplanet dismissed limebell’s stale review via 0c9f50a June 5, 2021 06:43

libplanet force-pushed the feat/txbindex branch from 5173022 to 0c9f50a Compare June 5, 2021 06:43

kfangw force-pushed the feat/txbindex branch from 0c9f50a to ad53a4d Compare June 7, 2021 01:29

kfangw marked this pull request as draft June 7, 2021 01:31

kfangw force-pushed the feat/txbindex branch 2 times, most recently from 0807d29 to cc49741 Compare June 7, 2021 01:34

kfangw marked this pull request as ready for review June 7, 2021 01:34

auto-assign bot requested review from dahlia and limebell June 7, 2021 01:34

dahlia previously approved these changes Jun 7, 2021

View reviewed changes

longfin reviewed Jun 7, 2021

View reviewed changes

kfangw added 5 commits June 8, 2021 17:23

build: ignore tests projects for codecov

1c17540

feat: introduce txid->blockhash index on IStore

55d652f

fix: remove chain id for the APIs

88a28f6

fix: change key

bc9e9ed

chore: update changelog

dc30fc4

kfangw force-pushed the feat/txbindex branch from 5f43072 to dc30fc4 Compare June 8, 2021 08:27

kfangw commented Jun 8, 2021

View reviewed changes

test: improve code coverage

3db3827

kfangw added 3 commits June 9, 2021 14:39

fix: introduce separated db for the index

cdf2e8d

chore: codecov ignore

5f51ee4

chore: ignore tests, benchmarks

0783fe8

longfin previously approved these changes Jun 14, 2021

View reviewed changes

limebell previously approved these changes Jun 14, 2021

View reviewed changes

Merge branch 'main' into feat/txbindex

a15bf0d

kfangw dismissed stale reviews from limebell and longfin via a15bf0d June 14, 2021 01:45

longfin requested review from dahlia, limebell and longfin June 14, 2021 01:55

longfin approved these changes Jun 14, 2021

View reviewed changes

kfangw merged commit eeaa686 into planetarium:main Jun 14, 2021

kfangw deleted the feat/txbindex branch June 14, 2021 02:05

longfin mentioned this pull request Jul 26, 2021

Apply IterateTxIdBlockHashIndex to Libplanet.Explorer #1399

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce txid->blockhash index on IStore #1328

feat: introduce txid->blockhash index on IStore #1328

kfangw commented Jun 3, 2021 •

edited

Loading

codecov bot commented Jun 3, 2021 •

edited

Loading

kfangw commented Jun 3, 2021

limebell left a comment

longfin commented Jun 5, 2021

longfin Jun 7, 2021

kfangw Jun 7, 2021

longfin Jun 7, 2021

kfangw commented Jun 7, 2021 •

edited

Loading

kfangw commented Jun 8, 2021

kfangw Jun 8, 2021

kfangw Jun 10, 2021

kfangw commented Jun 9, 2021

kfangw commented Jun 14, 2021

longfin commented Jun 14, 2021

longfin left a comment

kfangw commented Jun 14, 2021

kfangw commented Jun 14, 2021

feat: introduce txid->blockhash index on IStore #1328

feat: introduce txid->blockhash index on IStore #1328

Conversation

kfangw commented Jun 3, 2021 • edited Loading

APIs

Contract

codecov bot commented Jun 3, 2021 • edited Loading

Codecov Report

kfangw commented Jun 3, 2021

limebell left a comment

Choose a reason for hiding this comment

longfin commented Jun 5, 2021

longfin Jun 7, 2021

Choose a reason for hiding this comment

kfangw Jun 7, 2021

Choose a reason for hiding this comment

longfin Jun 7, 2021

Choose a reason for hiding this comment

kfangw commented Jun 7, 2021 • edited Loading

kfangw commented Jun 8, 2021

kfangw Jun 8, 2021

Choose a reason for hiding this comment

kfangw Jun 10, 2021

Choose a reason for hiding this comment

kfangw commented Jun 9, 2021

kfangw commented Jun 14, 2021

longfin commented Jun 14, 2021

longfin left a comment

Choose a reason for hiding this comment

kfangw commented Jun 14, 2021

kfangw commented Jun 14, 2021

kfangw commented Jun 3, 2021 •

edited

Loading

codecov bot commented Jun 3, 2021 •

edited

Loading

kfangw commented Jun 7, 2021 •

edited

Loading