AbiCoder to decode struct of private mapping intermittent issues

[Brando753]

First, I saw the other discussion, which was very similar to this one here #2373 however, that was in regards to other variables being packed unrelated to the current struct. Because this is dynamically allocated storage, that should not be an issue.

I have written a base hardhat project for doing the Ethernaut challenges, and one of the things I wrote was a function called fetchInstance which would automatically find your instance address based on the level address. Unfortunately, because of how OpenZeppelin wrote their Ethernaut contract, I have to read private storage to do this. There is a mapping of addresses to a struct

    struct EmittedInstanceData {
        address player;
        Level level;
        bool completed;
    }

    mapping(address => EmittedInstanceData) emittedInstances;

and I used the AbiCoder to decode the struct. The code I wrote initially was working fine for both completed and incomplete levels but suddenly, it started erroring out for certain instances. I have included a minimum example of a good and bad instance that I am trying to decode.

Here is that example code:

import { ethers } from "hardhat";

// Address of Ethernaut Control Contract on Goerli Test Network
const ETHERNAUT_ADDRESS = "0x42E7014a9D1f6765e76fA2e69532d808F2fe27E3";

// Address that decodes fine
const NORMAL_INSTANCE = "0x003a808Dbe736441932953c48DdfD4cD6f013ed9";

// Address of Instance that has a bug
const BUGGY_INSTANCE = "0x242CB4b74caAf7570c682474E9F76B5Ef0f4f9a8";

// Typings for Internal EmittedInstanceData structure used in private variable
// of Ethernaut Control Contract.
interface EmittedInstanceData {
  player: string;
  level: string;
  completed: boolean;
}

/**
 * Function to read the private mapping data emittedInstances because OpenZeppelin
 * did not care to make finding your current instances very easy did they?
 * @param instanceAddress The instance you are looking up data for to map to a specific level
 * @returns Returns an EmittedInstanceData object
 */
async function FetchEmittedInstances(
  instanceAddress: string
): Promise<EmittedInstanceData> {
  // The keyslot for emittedInstances is 2 with 0 held by the owner address and 1 held by registeredLevels
  const abiCoder = new ethers.utils.AbiCoder();
  // Mapping Key Slot is computed as keccak256(instanceAddress, keySlot) where both are fully padded values
  const storageSlot = ethers.BigNumber.from(
    ethers.utils.keccak256(
      ethers.utils.concat([
        abiCoder.encode(["address"], [instanceAddress]), // They key of the map
        abiCoder.encode(["uint256"], [2]), // The keyslot of the mapping
      ])
    )
  );
  // Fetch storage for internal data structure at the calculated key slot
  const storage1Promise = ethers.provider.getStorageAt(
    ETHERNAUT_ADDRESS,
    storageSlot
  );
  const storage2Promise = ethers.provider.getStorageAt(
    ETHERNAUT_ADDRESS,
    storageSlot.add(1)
  );
  // I am unsure why we need a third storage slot value as the ABI should have
  // packed the address and bool values in the same slot but it errors if we
  // don't feed it a third slot...
  const storage3Promise = ethers.provider.getStorageAt(
    ETHERNAUT_ADDRESS,
    storageSlot.add(2)
  );
  const [storage1, storage2, storage3] = await Promise.all([
    storage1Promise,
    storage2Promise,
    storage3Promise,
  ]);
  // Concat the three byte32 values into one single data blob
  const storageData = ethers.utils.concat([storage1, storage2, storage3]);
  // Decode the dataBlob into the types address (string) and bool (boolean)
  const result = abiCoder.decode(
    ["tuple(address,address,bool)"],
    storageData
  )[0];
  // Return typed as the interface EmmittedInstanceData
  return {
    player: result[0],
    level: result[1],
    completed: result[2],
  };
}

async function main() {
  console.log(await FetchEmittedInstances(NORMAL_INSTANCE));
  console.log(await FetchEmittedInstances(BUGGY_INSTANCE));
}

main()
  .then(() => console.log("Script completed."))
  .catch((error) => console.error("Error Encountered: ", error));

So two questions, what is the proper way to decode that struct, and second why does it need three bytes32 strings when it should be entirely packed in two storage slots?

[ricmoo]

Unfortunately, you likely cannot do things this way.

The struct structure used in storage is not necessarily ABI encoded, so simply fetching those values will generally not work in all cases. You will need to either use (and possibly add) getters to the contract or replicate all the complex Solidity decoding yourself.

Here is the info on how bytes and strings are placed in storage: https://docs.soliditylang.org/en/v0.8.17/internals/layout_in_storage.html?highlight=Storage#bytes-and-string

This is because the cost of storage is astronomical, so a little extra logic to save a word here and there matter. But it does mean you need a translation layer, which solidity automatically creates.

Does that make sense? And answer the question?

[ricmoo]

It packs and unpacks ABI encoded data, such as what the tx.data is and what is returned from an eth_call.

Storage in the state true is encoded using its own format, by chance it is similar to ABI encodings, sometimes. But the storage is up to the compiler code to choose how to store and read it.

Think of it like a PNG; on disk it is in PNG format, but when a program opens it to show it on-screen, it is up to the image program how to store it in memory, for example it may separate the image into layers, add additional bytes for alpha channels, etc.

A contract’s storage is more like the memory in the PNG example; the program gets to decide how to store it. But once it is serialized via a getter, it is assembled into ABI encoded data.

[ricmoo]

(also, it has nothing to do with “most efficient”; it follows the established public specification for how ABI encoded data works, this way it works regardless of whether you are using Solidity, Vyper, JavaScript, Rust or whatever. It’s a standard serialization that everyone can read and write. ;))

[ricmoo]

(reading storage slots like you are in your example would be like trying to read random pieces of a running program’s memory and trying to make sense of it; often it will work, but often it won’t. So we force applications to have a sane API we can extract the meaning out of it, using agreed encodings)

[Brando753]

Ah yes, I just assumed they packed data for storage and message-sending in a way that used the least data but that may have been a mistake. So the AbiCoder is for the specification of call data only and the storage specification is different. Is there a decoder for storage layout? The format in solidity is guaranteed, which is why upgradable contracts with the proxy pattern are possible.

The documentation has the following quote:

The layout of state variables in storage is considered to be part of the external interface of Solidity due to the fact that storage pointers can be passed to libraries. This means that any change to the rules outlined in this section is considered a breaking change of the language and due to its critical nature should be considered very carefully before being executed. In the event of such a breaking change, we would want to release a compatibility mode in which the compiler would generate bytecode supporting the old layout.

It would seem that just like we have AbiCoder we could have a similar one for storage and if the storage packing ever changed we could specify the version of storage layout.

[Brando753]

So I have updated my code, and it appears to work. I have now used hexslice to split up the values into fully padded hexstrings for each data type and then fed those values into AbiCoder and that appears to work. I would like to know if there is any proper way to parse storage values and if not, could one be added as a feature to this library as storage layout by solidity appears to be well-defined and unlikely to change. I imagine I am not the only one who would like more convenient ways to access private values in storage.

New code:

  const playerAddressBinary = storage1;
  const levelAddressBinary = ethers.utils.hexZeroPad(
    ethers.utils.hexDataSlice(storage2, 12, 32),
    32
  );

  const levelCompletionBinary = ethers.utils.hexZeroPad(
    ethers.utils.hexDataSlice(storage2, 11, 12),
    32
  );
  // Concat the three byte32 values into one single data blob
  const storageData = ethers.utils.concat([
    playerAddressBinary,
    levelAddressBinary,
    levelCompletionBinary,
  ]);
  // Decode the dataBlob into the types address (string) and bool (boolean)
  const result = abiCoder.decode(["address", "address", "bool"], storageData);