Document method for reviewers to verify chainTxData #12317

jnewbery commented at 6:50 PM on January 31, 2018: member

This commit adds the final block hash of the window to getchaintxstats and documents how reviewers can verify changes to chainTxData.

MarcoFalke commented at 6:55 PM on January 31, 2018: member

The block hash is shown in the diff on the pull request. I think that implies that you as a reviewer go ahead and type in "getchaintxstats (30 * 24 * 6) $that_hash". No need to be overly verbose, imo.

In the worst case they would run getchaintxstats without any args and get a similar result.

MarcoFalke added the label Docs on Jan 31, 2018

jnewbery commented at 7:04 PM on January 31, 2018: member

In the worst case they would run getchaintxstats without any args and get a similar result.

Here's @laanwj 's getchaintxstats from #12270:

{
  "time": 1516903077,
  "txcount": 295363220,
  "window_block_count": 4320,
  "window_tx_count": 7882762,
  "window_interval": 2242748,
  "txrate": 3.514778298765621
}

and mine when I check it a few days later:

→ bcli getchaintxstats
{
  "time": 1517425124,
  "txcount": 296682224,
  "window_block_count": 4320,
  "window_tx_count": 7270004,
  "window_interval": 2273816,
  "txrate": 3.197270139712272
}

so txrate is ~10% different. I didn't ACK that PR because I didn't think that there was a way to verify it.

MarcoFalke commented at 10:01 PM on January 31, 2018: member

Maybe we should take the max for txrate, since it fluctuates so much. Or take a larger interval for that?

jnewbery commented at 10:13 PM on January 31, 2018: member

Maybe we should take the max for txrate, since it fluctuates so much. Or take a larger interval for that?

Perhaps, but I think that's a separate issue from this.

I'm just looking for a way that reviewers can quickly and easily verify the update to chainTxData.

laanwj commented at 8:11 AM on February 1, 2018: member

utACK, TIL that getchaintxstats takes arguments at all.

in src/rpc/blockchain.cpp:1534 in 0d0bef50b4 outdated

1535 | +            "  \"time\": xxxxx,                         (numeric) The timestamp for the final block in the window in UNIX format.\n"
1536 | +            "  \"txcount\": xxxxx,                      (numeric) The total number of transactions in the chain up to that point.\n"
1537 | +            "  \"window_block_count\": xxxxx,           (numeric) Size of the window in number of blocks.\n"
1538 | +            "  \"window_tx_count\": xxxxx,              (numeric) The number of transactions in the window. Only returned if \"window_block_count\" is > 0.\n"
1539 | +            "  \"window_interval\": xxxxx,              (numeric) The elapsed time in the window in seconds. Only returned if \"window_block_count\" is > 0.\n"
1540 | +            "  \"window_final_block_hash\": \"...\",      (string) the hash of the final block in the window\n"

promag commented at 12:20 PM on February 2, 2018:

Sentence case. Missing period.

promag commented at 12:27 PM on February 2, 2018: member

Could have 2 tests:

window_final_block_hash is in the response and it's a block hash
window_final_block_hash matches blockhash parameter when specified.

Document method for reviewers to verify chainTxData

This commit adds the final block hash of the window to getchaintxstats
and documents how reviewers can verify changes to chainTxData.

7444149de3

jnewbery force-pushed on Feb 2, 2018

jnewbery commented at 2:30 PM on February 2, 2018: member

Addressed @promag nits

promag commented at 3:08 PM on February 2, 2018: member

utACK 7444149.

jamesob commented at 4:37 PM on February 2, 2018: member

utACK https://github.com/bitcoin/bitcoin/pull/12317/commits/7444149de37a0cce9b28ca92f0af16cac8689873

laanwj merged this on Feb 2, 2018

laanwj closed this on Feb 2, 2018

laanwj referenced this in commit 85123be78d on Feb 2, 2018

jnewbery deleted the branch on Feb 2, 2018

meshcollider commented at 7:48 PM on February 2, 2018: contributor

Post-merge-utACK https://github.com/bitcoin/bitcoin/pull/12317/commits/7444149de37a0cce9b28ca92f0af16cac8689873

jasonbcox referenced this in commit 129395a132 on Apr 5, 2019

jonspock referenced this in commit ad462f144d on Apr 7, 2019

jonspock referenced this in commit dbc2b56363 on Apr 8, 2019

jonspock referenced this in commit af4f8fee91 on Apr 8, 2019

jonspock referenced this in commit e41df7d91d on Apr 8, 2019

PastaPastaPasta referenced this in commit 847d920623 on Jun 9, 2020

PastaPastaPasta referenced this in commit 2e12e1a2be on Jun 9, 2020

PastaPastaPasta referenced this in commit 8c4b6a40db on Jun 10, 2020

PastaPastaPasta referenced this in commit 4de0852081 on Jun 10, 2020

PastaPastaPasta referenced this in commit 3af9ef9194 on Jun 11, 2020

PastaPastaPasta referenced this in commit 4afcc5b947 on Jun 11, 2020

DrahtBot locked this on Sep 8, 2021