-
Notifications
You must be signed in to change notification settings - Fork 683
fix: set difficulty
default to 1 and add totalDifficulty
#771
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really Awesome work. I committed a change, if you wouldn't mind reviewing it.
The gist of the change I made was because:
totalDifficulty
shouldn't be included in block size calculationstotalDifficulty
shouldn't be included in the block hash (which it previously was, because it was included in theraw
array)
It seems to work now, and as a bonus, I figured out why our block size calculations were always off a little (PR for that coming soon!)
I also requested some more tests :-D
Thanks
Quantity.from(timestamp) | ||
Quantity.from(timestamp), | ||
this.#options.miner.difficulty, | ||
Quantity.from(0) // we start the totalDifficulty at 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
Quantity.from(0) // we start the totalDifficulty at 0 | |
RPCQUANTITY_ZERO // we start the totalDifficulty at 0 |
await provider.send("eth_subscribe", ["newHeads"]); | ||
|
||
await provider.send("eth_sendTransaction", [{ from, to: from }]); | ||
|
||
await provider.once("message"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't need the transaction itself, you can do await provider.send("evm_mine")
instead.
Oh, actually... now that #793 is merged (as of yesterday), you can do await provider.send("evm_mine", [{blocks: number}])
, no (backwards 😄) loop needed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh cool! I'll merge in develop
and update this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like deleting code.
const block = await provider.send("eth_getBlockByNumber", ["0x0"]); | ||
assert.strictEqual( | ||
block.totalDifficulty, | ||
`0x${DEFAULT_DIFFICULTY}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If DEFAULT_DIFFICULTY
is ever greater than 9 in the future this test will fail.
Other ways of solving this:
assert.strictEqual(
block.totalDifficulty,
Quantity.from(DEFAULT_DIFFICULTY).toString()
);
or
assert.strictEqual(
parseInt(block.totalDifficulty, 16),
DEFAULT_DIFFICULTY
);
or
assert.strictEqual(
BigInt(block.totalDifficulty),
DEFAULT_DIFFICULTY // change DEFAULT_DIFFICULTY to a BigInt
);
and lots of others, I'm sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also update other places that use the 0x{number}
pattern in these tests? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I meant to update these but must have overlooked them. Updated!
f27b613
to
609c25c
Compare
… current block's difficulty
I had to store totalDifficulty in a different way so it wasn't included in the `raw` fields (which messes with the hash).
Co-authored-by: David Murdoch <[email protected]>
Co-authored-by: David Murdoch <[email protected]>
609c25c
to
1c90485
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two more little "suggestions" and it's good to go!
src/chains/ethereum/ethereum/tests/api/eth/getBlockByNumber.test.ts
Outdated
Show resolved
Hide resolved
src/chains/ethereum/ethereum/tests/api/eth/getBlockByNumber.test.ts
Outdated
Show resolved
Hide resolved
difficulty
and totalDifficulty
difficulty
default to 1 and add totalDifficulty
…suite#771) Co-authored-by: David Murdoch <[email protected]>
Currently Ganache sets all block headers to have
difficulty
andtotalDifficulty
hard-coded to 0. This PR causes Ganache to set the difficulty of each block (by default) to 1 (the user can specify their own block difficulty on the command line). ThetotalDifficulty
is also now calculated based on the previous block'stotalDifficulty
and the block difficulty.