Add BlockTransaction.Index and BlockOperation.Index

[anarcher]

Github Issue

Background

Like Block.Height, BlockTransaction and BlockOperation can have their own ordered index.
The advantage of the indexes is that 1) make readable ordering for DB index and cursor, 2) we can easily realize like that the Second op of 21 tx of 1202 height.. and all nodes have same indexes.

Background: All nodes have the same indexes if nodes have the same hash.

Solution

• Add Index field to BlockTransaction and BlockOperation
• Add index field to api/resource/BlockTransaction and api/resource/BlockOperation
• Test codes
• Make DB Keys of BlockTransaction and BlockOperation.
• PageCursor
• Support prev and next using one more thing....

Possible Drawbacks

[anarcher]

$http localhost:2821/api/v1/blocks/162 | jq .transactions
[
  "G6dxdjTcn4DG3F2BXR52udqcnMSx38X2BUECGZFQpXo",
  "Bjb5LXkdSoWWzBb6AJzxkfpv3KtA78LpXkqUxMNxRijz",
  "BbJHTfEvUEppUFq1QZYmZUu5nA3RPd5v1JcBw9K4b9Rs",
  "eXkEV1niwq7d78jvNaVnvAWdggAozFWsScG3VaryHrv",
  "AH2D27NvKKUNHeaRFbNq3FBUDt6MnbooNGUkGrH1zrY5",
  "5T1EVPwK1RoBBBst3SrSLJVaQP6vNpRJuRdWhdVKs57n",
  "4W7j9uEx6sdYewq7qbD2PNFgap39YyVj5szJkx5WeBGn",
  "CNP2Xwh89sUhWCbCfcDmfjF8EgvHeHij3bqWeMFnsAJD",
  "BXmLKMvMbRru53tbmnDNreJbTz77PwXNXjhJnrPm2Cs5",
  "BfvBxeBr142mDYZkjgiBzn8xnS1R8D84zXwHvDPsa1rY"
]
$http localhost:2823/api/v1/transactions/AH2D27NvKKUNHeaRFbNq3FBUDt6MnbooNGUkGrH1zrY5 | jq .index
5
http localhost:2823/api/v1/transactions/AH2D27NvKKUNHeaRFbNq3FBUDt6MnbooNGUkGrH1zrY5/operations | jq ._embedded.records[].index
0

http localhost:2823/api/v1/transactions/AH2D27NvKKUNHeaRFbNq3FBUDt6MnbooNGUkGrH1zrY5
HTTP/1.1 200 OK
Content-Length: 571
Content-Type: application/hal+json
Date: Tue, 20 Nov 2018 05:53:22 GMT
X-Ratelimit-Limit:
X-Ratelimit-Remaining:
X-Ratelimit-Reset:

{
    "_links": {
        "account": {
            "href": "/api/v1/accounts/GBO5VCIB3CRFZIGMMOPFXZ6AY4FTLKW4UK2AW22DIQ6X5XLQDFWBEVGG"
        },
        "operations": {
            "href": "/api/v1/transactions/AH2D27NvKKUNHeaRFbNq3FBUDt6MnbooNGUkGrH1zrY5/operations{?cursor,limit,order}",
            "templated": true
        },
        "self": {
            "href": "/api/v1/transactions"
        }
    },
    "block": "FtZYPHRxbnbRQq1bi1hkbbzGGVW1JK2hwwGi7qJhGYuM",
    "created": "2018-11-20T14:35:27.125297000+09:00",
    "fee": "10000",
    "hash": "AH2D27NvKKUNHeaRFbNq3FBUDt6MnbooNGUkGrH1zrY5",
    "index": 5,
    "operation_count": 1,
    "sequence_id": 39,
    "source": "GBO5VCIB3CRFZIGMMOPFXZ6AY4FTLKW4UK2AW22DIQ6X5XLQDFWBEVGG"
}

[codecov-io]

Codecov Report

Merging #765 into master will decrease coverage by 0.19%.
The diff coverage is 77.77%.

@@           Coverage Diff            @@
##           master    #765     +/-   ##
========================================
- Coverage   60.49%   60.3%   -0.2%     
========================================
  Files         153     156      +3     
  Lines       10708   10859    +151     
========================================
+ Hits         6478    6548     +70     
- Misses       3498    3572     +74     
- Partials      732     739      +7

Flag	Coverage Δ
#integration_tests_long_term	44.21% <54.67%> (-0.34%)	⬇️
#integration_tests_node	40.21% <54.67%> (-0.16%)	⬇️
#unittests	48.57% <70.46%> (-0.09%)	⬇️

Impacted Files	Coverage Δ
lib/node/runner/api/api.go	62.96% <ø> (+2.24%)	⬆️
lib/block/test.go	75.67% <100%> (ø)	⬆️
lib/common/util.go	33.73% <100%> (-1.56%)	⬇️
lib/block/block.go	48.83% <100%> (ø)	⬆️
lib/node/runner/block_operations.go	64.73% <100%> (+1.05%)	⬆️
lib/node/runner/api/resource/transaction.go	59.45% <100%> (+2.31%)	⬆️
lib/block/genesis.go	79.68% <100%> (ø)	⬆️
lib/node/runner/api/resource/operation.go	83.33% <100%> (+0.57%)	⬆️
lib/node/runner/finish_ballot.go	43.75% <100%> (ø)	⬆️
lib/block/order.go	48% <48%> (ø)
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5c5b761...185a0a6. Read the comment docs.

[kfangw]

As you can see here

sebak/lib/block/operation.go

Lines 51 to 53 in 820e28d

	return BlockOperation{
	Hash: NewBlockOperationKey(opHash, txHash),

BlockOperation's Hash is a combination of TxHash and OpHash. BecauseOpHash of BlockOperation is not unique.
Let's assume that Account1 pay 1000 amount to Account2. And after that, Account1 pay 1000 amount to Account2 again. In this case, the first operation and the second operation is exactly the same and the hashes also the same. Hence, OpHash is not unique.

To overcome this, As-is implementation uses the combination of TxHash and OpHash for unique ID of BlockOperation.

Different from others such as Block and Transaction, Operation's Hash is just an index key for the database.

In conclusion, therefore, if this PR support index key as a combination of the sequence number of Block, Transaction, and Operation, then we can replace the Hash of operation with the index key
And the Hash is no longer needed to be exposed to a client by API.

@anarcher What do you think about this?

[anarcher]

@kfangw IMHO, the ophash is proof of validating an operation's payload. So I am not entirely sure but Exposing ophash to API is not bad. But The /api/v1/transactions/{txHash}/operations/{opHash} is unique location. Or Supporting /api/v1/transactions/{txHash}/operations/1 ? haha...

With #767 , My original idea is to use Tx.Index and Op.Index for ordering part for cursor (not prefixing part).
But I am not sure that we can use it for prefixing part. Such as {height}-{tx.Index}-{op.Index} humm...

[anarcher]

@outsideris @kfangw Thanks for kindly discuss. I feel that I understand this opHash issues.

[anarcher]

$http localhost:2822/api/v1/transactions limit==100 | jq -c "._embedded.records[] | [ .block , .index ]"
["59EKzLpsAbmJGFT3DmCL9rt6M3WQz8CRhJU7mkejW53N",8]
["59EKzLpsAbmJGFT3DmCL9rt6M3WQz8CRhJU7mkejW53N",9]
["59EKzLpsAbmJGFT3DmCL9rt6M3WQz8CRhJU7mkejW53N",10]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",0]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",1]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",2]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",3]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",4]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",5]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",6]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",7]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",8]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",9]
["2XBxxbhfbwPPoQ6RFDciEKGtGdicmRr4MvxdqztJQcbw",10]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",0]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",1]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",2]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",3]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",4]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",5]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",6]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",7]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",8]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",9]
["DB9d94gywvmTTmnem9ECPYREoGAuEAK9EYc5wZPA6uK",10]
["4aD7QvZsGR4XyXqkafhGJn7wojTrCkHYBbCTCWWLVM6R",0]

[anarcher]

$http localhost:2822/api/v1/transactions cursor==20-0 | jq -cr "._embedded.records[] | [ .block_height,.index] "
[20,0]
[21,0]
[22,0]
[23,0]
[24,0]
[25,0]
[26,0]
[27,0]
[28,0]
[29,0]
[30,0]
[31,0]
[32,0]
[33,0]
[34,0]
[35,0]
[36,0]
[37,0]
[38,0]
[39,0]
(go:bos) anarch@chainer ~/go/bos/src/sebak-hot-body (master)
$http localhost:2822/api/v1/transactions cursor==20-0 | jq -cr "._links"
{"next":{"href":"/api/v1/transactions?cursor=39-0&limit=20&reverse=false"},"prev":{"href":"/api/v1/transactions?cursor=39-0&limit=20&reverse=true"},"self":{"href":"/api/v1/transactions?cursor=20-0"}}

What do you think about it? @spikeekips @kfangw @Charleslee522

[Geod24]

Why does the BlockOperation store the tx index ?

[anarcher]

I think that total order of operation is height-txindex-opindex. So..

[kfangw]

So far, so good.
When you finished, let me know.
@anarcher

[anarcher]

I almost finished this PR.... 💤

[anarcher]

I finished it. plz review it....

[anarcher]

I revert BlockIndexes.
How do I do about this PR? ㅠ

[anarcher]

type Index interface {
    Key() string  //  key = {prefix}/{order}
    Prefix() string // {prefix}/
    Order() string  // order = {prefix}/{order}...  // order is parts of key or key
}

How about this interface?

BlockXXX.Save() and GetXXX() has a relation with index(key,prefix,order). (key and prefix and order). But the current design has not. It makes confusing to read when an object has more indexes