Skip to content

Commit

Permalink
Merge pull request #2581 from NomicFoundation/francovictorio/hh-321
Browse files Browse the repository at this point in the history 
Make eth_getStorageAt spec-compliant
  • Loading branch information
@fvictorio
fvictorio authored May 12, 2022
2 parents 0946d93 + 83914cb commit 7066f8c
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-kings-destroy.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"hardhat": patch
---

Make `eth_getStorageAt` spec-compliant. This means that the storage slot argument **must** have a length of 32 bytes (a hex-encoded string of length 66).
44 changes: 44 additions & 0 deletions packages/hardhat-core/src/internal/core/jsonrpc/types/base-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,41 @@ export const rpcHash = new t.Type<Buffer>(
t.identity
);

export const rpcStorageSlot = new t.Type<BN>(
"Storage slot",
BN.isBN,
validateStorageSlot,
t.identity
);

function validateStorageSlot(u: unknown, c: t.Context): t.Validation<BN> {
if (typeof u !== "string") {
return t.failure(
u,
c,
`Storage slot argument must be a string, got '${u as any}'`
);
}

if (u.match(/^0x(?:[0-9a-fA-F]*)*$/) === null) {
return t.failure(
u,
c,
`Storage slot argument must be a valid hexadecimal prefixed with "0x", got '${u}'`
);
}

if (u.length !== 66) {
return t.failure(
u,
c,
`Storage slot argument must have a length of 66 ("0x" + 32 bytes), but '${u}' has a length of ${u.length}`
);
}

return t.success(new BN(toBuffer(u)));
}

export const rpcAddress = new t.Type<Buffer>(
"ADDRESS",
(v): v is Buffer => Buffer.isBuffer(v) && v.length === ADDRESS_LENGTH_BYTES,
Expand Down Expand Up @@ -88,6 +123,15 @@ export function numberToRpcQuantity(n: number | BN): string {
return `0x${n.toString(16)}`;
}

export function numberToRpcStorageSlot(n: number | BN): string {
assertHardhatInvariant(
typeof n === "number" || BN.isBN(n),
"Expected number"
);

return `0x${n.toString(16).padStart(64, "0")}`;
}

/**
* Transforms a DATA into a number. It should only be used if you are 100% sure that the data
* represents a value fits in a number.
Expand Down
3 changes: 2 additions & 1 deletion packages/hardhat-core/src/internal/hardhat-network/provider/modules/eth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
rpcFloat,
rpcHash,
rpcQuantity,
rpcStorageSlot,
} from "../../../core/jsonrpc/types/base-types";
import {
optionalRpcNewBlockTag,
Expand Down Expand Up @@ -694,7 +695,7 @@ export class EthModule {
return validateParams(
params,
rpcAddress,
rpcQuantity,
rpcStorageSlot,
optionalRpcNewBlockTag
);
}
Expand Down
95 changes: 80 additions & 15 deletions ...s/hardhat-core/test/internal/hardhat-network/provider/modules/eth/methods/getStorageAt.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import { assert } from "chai";

import { numberToRpcQuantity } from "../../../../../../../src/internal/core/jsonrpc/types/base-types";
import {
numberToRpcQuantity,
numberToRpcStorageSlot,
} from "../../../../../../../src/internal/core/jsonrpc/types/base-types";
import { workaroundWindowsCiFailures } from "../../../../../../utils/workaround-windows-ci-failures";
import { assertInvalidArgumentsError } from "../../../../helpers/assertions";
import { EXAMPLE_CONTRACT } from "../../../../helpers/contracts";
import { setCWD } from "../../../../helpers/cwd";
import {
Expand Down Expand Up @@ -39,23 +43,23 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(3),
numberToRpcStorageSlot(3),
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
);

assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(4),
numberToRpcStorageSlot(4),
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
);

assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
DEFAULT_ACCOUNTS_ADDRESSES[0],
numberToRpcQuantity(0),
numberToRpcStorageSlot(0),
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
);
Expand All @@ -74,7 +78,7 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
numberToRpcQuantity(firstBlock),
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
Expand All @@ -83,15 +87,15 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
]),
"0x1234567890123456789012345678901234567890123456789012345678901234"
);

assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
"latest",
]),
"0x1234567890123456789012345678901234567890123456789012345678901234"
Expand Down Expand Up @@ -126,15 +130,15 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
contractAddress,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
);

assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
contractAddress,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
"pending",
]),
"0x1234567890123456789012345678901234567890123456789012345678901234"
Expand All @@ -150,7 +154,7 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
"latest",
]),
"0x1234567890123456789012345678901234567890123456789012345678901234"
Expand All @@ -159,7 +163,7 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(2),
numberToRpcStorageSlot(2),
"earliest",
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
Expand Down Expand Up @@ -192,7 +196,7 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(0),
numberToRpcStorageSlot(0),
numberToRpcQuantity(firstBlock + 1),
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
Expand All @@ -201,7 +205,7 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(0),
numberToRpcStorageSlot(0),
]),
"0x000000000000000000000000000000000000000000000000000000000000007b"
);
Expand All @@ -220,7 +224,7 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(0),
numberToRpcStorageSlot(0),
numberToRpcQuantity(firstBlock + 2),
]),
"0x000000000000000000000000000000000000000000000000000000000000007b"
Expand All @@ -229,14 +233,75 @@ describe("Eth module", function () {
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
exampleContract,
numberToRpcQuantity(0),
numberToRpcStorageSlot(0),
]),
"0x000000000000000000000000000000000000000000000000000000000000007c"
);
});
});
});
});

describe("validation", function () {
it("should accept valid storage slot arguments", async function () {
// 0x010101... is almost surely an empty account
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
"0x0101010101010101010101010101010101010101",
"0x0000000000000000000000000000000000000000000000000000000000000000",
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
);

// check that it also works with some random storage slot
assert.strictEqual(
await this.provider.send("eth_getStorageAt", [
"0x0101010101010101010101010101010101010101",
"0xcd39aa866fd639607c7241f617cf83f33c646551f3d205f2905c5abacca2db85",
]),
"0x0000000000000000000000000000000000000000000000000000000000000000"
);
});

it("should not accept plain numbers", async function () {
await assertInvalidArgumentsError(
this.provider,
"eth_getStorageAt",
["0x0101010101010101010101010101010101010101", 0],
"Storage slot argument must be a string, got '0'"
);
});

it("should not accept invalid hex strings", async function () {
await assertInvalidArgumentsError(
this.provider,
"eth_getStorageAt",
["0x0101010101010101010101010101010101010101", "0xABCDEFG"],
"Storage slot argument must be a valid hexadecimal prefixed with \"0x\", got '0xABCDEFG'"
);
});

it("should not accept hex strings that are too short", async function () {
await assertInvalidArgumentsError(
this.provider,
"eth_getStorageAt",
["0x0101010101010101010101010101010101010101", "0x0"],
`Storage slot argument must have a length of 66 ("0x" + 32 bytes), but '0x0' has a length of 3`
);
});

it("should not accept hex strings that are too long", async function () {
await assertInvalidArgumentsError(
this.provider,
"eth_getStorageAt",
[
"0x0101010101010101010101010101010101010101",
"0x00000000000000000000000000000000000000000000000000000000000000000",
],
`Storage slot argument must have a length of 66 ("0x" + 32 bytes), but '0x00000000000000000000000000000000000000000000000000000000000000000' has a length of 67`
);
});
});
});
});
});
Expand Down
5 changes: 3 additions & 2 deletions packages/hardhat-core/test/internal/hardhat-network/provider/modules/hardhat.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import sinon from "sinon";
import { describe } from "mocha";
import {
numberToRpcQuantity,
numberToRpcStorageSlot,
rpcQuantityToBN,
rpcQuantityToNumber,
} from "../../../../../src/internal/core/jsonrpc/types/base-types";
Expand Down Expand Up @@ -2068,7 +2069,7 @@ describe("Hardhat module", function () {

const resultingStorageValue = await this.provider.send(
"eth_getStorageAt",
[DEFAULT_ACCOUNTS_ADDRESSES[0], numberToRpcQuantity(0), "latest"]
[DEFAULT_ACCOUNTS_ADDRESSES[0], numberToRpcStorageSlot(0), "latest"]
);

assert.equal(resultingStorageValue, targetStorageValue);
Expand Down Expand Up @@ -2169,7 +2170,7 @@ describe("Hardhat module", function () {
assert.equal(
await this.provider.send("eth_getStorageAt", [
DEFAULT_ACCOUNTS_ADDRESSES[0],
numberToRpcQuantity(0),
numberToRpcStorageSlot(0),
currentBlockNumber,
]),
targetStorageValue
Expand Down

0 comments on commit 7066f8c

Please sign in to comment.