-
Notifications
You must be signed in to change notification settings - Fork 771
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.
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
block: remove isTruthy and isFalsy #2256
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
const { | ||
parentHash, | ||
sha3Uncles, | ||
miner, | ||
stateRoot, | ||
transactionsRoot, | ||
receiptRoot, | ||
receiptRoot, // TODO: Investigate dual receiptRoot/receiptsRoot usage. |
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.
This TODO can be removed, right?
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.
Yep, done by merging with master
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 points about types.
packages/block/test/block.spec.ts
Outdated
@@ -176,7 +176,7 @@ tape('[Block]: block functions', function (t) { | |||
const common = new Common({ chain: Chain.Goerli, hardfork: Hardfork.Chainstart }) | |||
|
|||
try { | |||
blockFromRpc(testDataFromRpcGoerli, [], { common }) | |||
blockFromRpc(testDataFromRpcGoerli as any, [], { common }) |
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 cast this as JsonRPCBlock
?
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.
yes! done
packages/block/test/from-rpc.spec.ts
Outdated
common, | ||
}) | ||
const blockDifficultyAsInteger = blockFromRpc( | ||
blockDataDifficultyAsInteger as any, |
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.
Same in this file, guess those any
s can be removed/changed?
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.
Indeed! done
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 CI passes then OK! Thanks!
Fixed, ready to approve @jochem-brouwer :) |
…hereumjs/ethereumjs-monorepo into block/remove-isTruthy-isFalsy
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.
LGTM with the noted changes. Let's merge it!
Extracts block from #2233