test: enable hardhat module tests #4721
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -1910,54 +2091,6 @@ mod tests { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| #[test] | |||
There was a problem hiding this comment.
I'm wondering, what's the reason for removing this test?
There was a problem hiding this comment.
When setting the nonce, we're now returning an error if there are any pending transactions (this is the behaviour in Hardhat). As a result, it's never possible for the mem pool to be updated
There was a problem hiding this comment.
I could move the check to the JSON-RPC layer, but it feels like a product design decision.
@alcuadrado & @fvictorio thoughts?
packages/hardhat-core/test/internal/hardhat-network/provider/modules/hardhat.ts
Show resolved
Hide resolved
packages/hardhat-core/test/internal/hardhat-network/provider/modules/hardhat.ts
Show resolved
Hide resolved
| `Storage key must not be greater than or equal to 2^256. Received ${ | ||
| isEdr | ||
| ? "0x10000000000000000000000000000000000000000000000000000000000000001" | ||
| : "115792089237316195423570985008687907853269984665640564039457584007913129639937" |
There was a problem hiding this comment.
Why are the values allowed to differ between EDR and Hardhat?
There was a problem hiding this comment.
This is something I wanted to discuss:
In JS/TS there is the BigInt type which is a smart integer type that can grow to any size. When this error is created by Hardhat, the incoming hexadecimal string is first parsed to a BigInt and then a check occurs to see whether the key is too large.
I wonder whether we should actually be printing the decimal notation of the key or the original hex string?
@fvictorio & @alcuadrado, I would appreciate your input.
If we should be returning a decimal number, we'd need to decode the hexadecimal value to a decimal string value, as there is no equivalent of the BigInt in our codebase.
There was a problem hiding this comment.
I think the user would expect the original hex string, as that's what we are telling them that is invalid.
There was a problem hiding this comment.
In that case, Hardhat would need to change its behaviour to return a hexadecimal string instead of the decimal one.
There was a problem hiding this comment.
I resolved it as follows: 052ebe8
packages/hardhat-core/test/internal/hardhat-network/provider/modules/hardhat.ts
Outdated
Show resolved
Hide resolved
No description provided.