Skip to content
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

transient storage consensus issue, Cancun #911

Closed
Tracked by #140
namiloh opened this issue Mar 21, 2024 · 8 comments
Closed
Tracked by #140

transient storage consensus issue, Cancun #911

namiloh opened this issue Mar 21, 2024 · 8 comments
Assignees
Labels
A-spec Area: specification C-bug Category: this is a bug, deviation, or other problem.

Comments

@namiloh
Copy link

namiloh commented Mar 21, 2024

Something related to TSTORE

Original finding

INFO [03-21|13:19:35.821] Shortcutting through abort
Consensus error
Testcase: /fuzztmp/00024752-mixed-1.json
- gethbatch-0: ./gethbatch-0-output.jsonl
  - command: /gethvm --json --noreturndata --nomemory statetest
- eelsbatch-0: ./eelsbatch-0-output.jsonl
  - command: /ethereum-spec-evm statetest --json --noreturndata --nomemory
- nethbatch-0: ./nethbatch-0-output.jsonl
  - command: /nethtest -x --trace -m
- besubatch-0: ./besubatch-0-output.jsonl
  - command: /evmtool/bin/evm --nomemory --notime --json state-test
- erigonbatch-0: ./erigonbatch-0-output.jsonl
  - command: /erigon_vm --json --noreturndata --nomemory statetest
- nimbus-0: ./nimbus-0-output.jsonl
  - command: /nimbvm --json --noreturndata --nomemory --nostorage /fuzztmp/00024752-mixed-1.json
- evmone-0: ./evmone-0-output.jsonl
  - command: /evmone --trace /fuzztmp/00024752-mixed-1.json
- revm-0: ./revm-0-output.jsonl
  - command: /revme statetest --json /fuzztmp/00024752-mixed-1.json
-------
prev:           both: {"depth":1,"pc":290,"gas":248898,"op":92,"opName":"TLOAD","stack":["0x4"]}
diff:    gethbatch-0: {"depth":1,"pc":291,"gas":248798,"op":80,"opName":"POP","stack":["0x0"]}
diff:    eelsbatch-0: {"depth":1,"pc":291,"gas":248798,"op":80,"opName":"POP","stack":["0x1"]}
INFO [03-21|13:19:36.110] Waiting for processes to exit

Explainer: they were both in agreement up to TLOAD, but in the next op they differed in the stack.
I have verified that eels is the odd one out, the others agree with geth.

Here is the minimized test:

{
  "00024752-mixed-1": {
    "env": {
      "currentCoinbase": "b94f5374fce5edbc8e2a8697c15331677e6ebf0b",
      "currentDifficulty": "0x200000",
      "currentRandom": "0x0000000000000000000000000000000000000000000000000000000000020000",
      "currentGasLimit": "0x26e1f476fe1e22",
      "currentNumber": "0x1",
      "currentTimestamp": "0x3e8",
      "previousHash": "0x044852b2a670ade5407e78fb2863c51de9fcb96542a07186fe3aeda6bb8a116d",
      "currentBaseFee": "0x10"
    },
    "pre": {
      "0x00000000000000000000000000000000000000f1": {
        "code": "0x7f7f6005600255600160045d6000600060006000600060065af1506000545060016000527f546000527f503a1a1cfa8877600560045d313b821d61600354506003545060036020527f5c506005606020526000604053605d60415360606042536000604353605c60446040527f53605060455360606046536002604753605c60485360506049536060604a536060605260206080536060608153604b6082536053608353606060845360606085536060608653604c608753605360885360606089536000608a536060608b53604d608c536053608d536060608e5360f3608f536060609053604e60915360536092536060609353604f6094536060609553600060965360f36097536000609860006000f560006000600060006000855af2505060045c50600760015d60005450600354506000600060006000600060f45af15060035450246000600060006000600060065af1507f7f767d78991c686f6d7f600160045560026004556003545060035450600160046000527f5d6000527f600160035560",
        "storage": {
          "0x0000000000000000000000000000000000000000000000000000000000000002": "0x0000000000000000000000000000000000000000000000000000000000000007"
        },
        "balance": "0x0",
        "nonce": "0x0"
      },
      "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
        "code": "0x",
        "storage": {},
        "balance": "0xffffffffff",
        "nonce": "0x0"
      }
    },
    "transaction": {
      "gasPrice": "0x10",
      "nonce": "0x0",
      "to": "0x00000000000000000000000000000000000000f1",
      "data": [
        "0x54209a9947898df95aec05ca1f6cb1d561b4211196025ee52cf812152c62e9b6aa348fadd9ac1095"
      ],
      "gasLimit": [
        "0x12c5f"
      ],
      "value": [
        "0x"
      ],
      "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
      "secretKey": "0x45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"
    },
    "out": "0x",
    "post": {
      "Cancun": [
        {
          "hash": "0x0000000000000000000000000000000000000000000000000000000000000000",
          "logs": "0x0000000000000000000000000000000000000000000000000000000000000000",
          "indexes": {
            "data": 0,
            "gas": 0,
            "value": 0
          }
        }
      ]
    }
  }
}

Geth ends with

{"pc":287,"op":80,"gas":"0x69","gasCost":"0x2","memSize":160,"stack":["0x6981f674fbaf9d6f293eaffc5855402eb64b8cbd"],"depth":1,"refund":0,"opName":"POP"}
{"pc":288,"op":96,"gas":"0x67","gasCost":"0x3","memSize":160,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":290,"op":92,"gas":"0x64","gasCost":"0x64","memSize":160,"stack":["0x4"],"depth":1,"refund":0,"opName":"TLOAD"}
{"pc":291,"op":80,"gas":"0x0","gasCost":"0x2","memSize":160,"stack":["0x0"],"depth":1,"refund":0,"opName":"POP","error":"out of gas"}
{"output":"","gasUsed":"0xd7d7","error":"out of gas"}
{"stateRoot": "0x7bfb83635d46a6250419c942b189a6dec350d140ec57cf6cb8030a4d26747441"}

eels ends with

{"pc":286,"op":80,"gas":"0x6b","gasCost":"0x2","memSize":160,"stack":["0x6981f674fbaf9d6f293eaffc5855402eb64b8cbd","0x0"],"depth":1,"refund":0,"opName":"POP"}
{"pc":287,"op":80,"gas":"0x69","gasCost":"0x2","memSize":160,"stack":["0x6981f674fbaf9d6f293eaffc5855402eb64b8cbd"],"depth":1,"refund":0,"opName":"POP"}
{"pc":288,"op":96,"gas":"0x67","gasCost":"0x3","memSize":160,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":290,"op":92,"gas":"0x64","gasCost":"0x64","memSize":160,"stack":["0x4"],"depth":1,"refund":0,"opName":"TLOAD"}
{"pc":291,"op":80,"gas":"0x0","gasCost":"0x2","memSize":160,"stack":["0x1"],"depth":1,"refund":0,"opName":"POP","error":"OutOfGasError"}
{"output":"","gasUsed":"0xd7d7","error":"OutOfGasError"}
{"stateRoot": "0x7bfb83635d46a6250419c942b189a6dec350d140ec57cf6cb8030a4d26747441"}

Note, I don't think the out-of-error exit is related. Before minimizing the testcase:

geth

{"pc":287,"op":80,"gas":"0x3cc47","gasCost":"0x2","memSize":160,"stack":["0x6981f674fbaf9d6f293eaffc5855402eb64b8cbd"],"depth":1,"refund":0,"opName":"POP"}
{"pc":288,"op":96,"gas":"0x3cc45","gasCost":"0x3","memSize":160,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":290,"op":92,"gas":"0x3cc42","gasCost":"0x64","memSize":160,"stack":["0x4"],"depth":1,"refund":0,"opName":"TLOAD"}
{"pc":291,"op":80,"gas":"0x3cbde","gasCost":"0x2","memSize":160,"stack":["0x0"],"depth":1,"refund":0,"opName":"POP"}
{"pc":292,"op":96,"gas":"0x3cbdc","gasCost":"0x3","memSize":160,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}

eels

{"pc":287,"op":80,"gas":"0x3cc47","gasCost":"0x2","memSize":160,"stack":["0x6981f674fbaf9d6f293eaffc5855402eb64b8cbd"],"depth":1,"refund":0,"opName":"POP"}
{"pc":288,"op":96,"gas":"0x3cc45","gasCost":"0x3","memSize":160,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":290,"op":92,"gas":"0x3cc42","gasCost":"0x64","memSize":160,"stack":["0x4"],"depth":1,"refund":0,"opName":"TLOAD"}
{"pc":291,"op":80,"gas":"0x3cbde","gasCost":"0x2","memSize":160,"stack":["0x1"],"depth":1,"refund":0,"opName":"POP"}
{"pc":292,"op":96,"gas":"0x3cbdc","gasCost":"0x3","memSize":160,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
@holiman
Copy link
Contributor

holiman commented Mar 21, 2024

Issue 911 too. Nice

@SamWilsn SamWilsn added A-spec Area: specification C-bug Category: this is a bug, deviation, or other problem. labels Mar 21, 2024
@holiman
Copy link
Contributor

holiman commented Mar 22, 2024

The original TSTORE happens via a delegatecall, btw

@holiman holiman closed this as completed Mar 22, 2024
@holiman
Copy link
Contributor

holiman commented Mar 22, 2024

Oops, wrong button

@holiman holiman reopened this Mar 22, 2024
@SamWilsn SamWilsn changed the title consensus issue, Cancun transient storage consensus issue, Cancun Mar 22, 2024
@gurukamath
Copy link
Collaborator

@holiman should be fixed in #912. Can be closed unless there are other outstanding issues.

@namiloh namiloh closed this as completed Mar 31, 2024
@holiman
Copy link
Contributor

holiman commented Apr 6, 2024

I've tried to repro this, in order to produce a statetest with a differing stateroot (so we can add it to the reference-tests). The naive way, is to

  1. A invokes B
  2. B does a tstore on a value
  3. B exits with error, e.g. revert
  4. A does a tload on the value, and
  5. puts it into the state via sstore.

However, I seem to be missing some nuance, because this simple flow does not trigger this. Could you please help me figure out what more is needed to trigger it ?

@gurukamath
Copy link
Collaborator

I've tried to repro this, in order to produce a statetest with a differing stateroot (so we can add it to the reference-tests).

I think the issue was that when a call was successful, we were not discarding the relevant snapshot (backup) of transient storage. So, when the parent call of the above mentioned call failed and we used the backups to restore, it was restoring the incorrect backup. So here is what I would suggest.

  1. A invokes B using callcode
  2. B does a tstore on a value and then invokes C
  3. C is successful
  4. B then fails with OutOfGasError
  5. A then does a tload on the value
  6. Puts the value from tload into state via sstore

@gurukamath
Copy link
Collaborator

gurukamath commented Apr 7, 2024

@holiman Starting with the original test from the fuzzer, I have put together a test that does this and stores the differentiating value to state using SSTORE. Hope this helps.

env

{
  "currentCoinbase": "b94f5374fce5edbc8e2a8697c15331677e6ebf0b",
  "currentDifficulty": "0x0",
  "currentRandom": "0x0000000000000000000000000000000000000000000000000000000000020000",
  "currentGasLimit": "0x26e1f476fe1e22",
  "currentNumber": "0x1",
  "currentTimestamp": "0x3e8",
  "previousHash": "0x044852b2a670ade5407e78fb2863c51de9fcb96542a07186fe3aeda6bb8a116d",
  "currentBaseFee": "0x10",
  "withdrawals": [],
  "parentBeaconBlockRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
  "currentExcessBlobGas": "0x0"
}

Transaction

[
  {
    "gasPrice": "0x10",
    "nonce": "0x0",
    "to": "0x00000000000000000000000000000000000000f1",
    "input": "0x54209a9947898df95aec05ca1f6cb1d561b4211196025ee52cf812152c62e9b6aa348fadd9ac1095",
    "gas": "0x12c5f",
    "value": "0x0",
    "sender": "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b",
    "secretKey": "0x45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8",
    "r": "0x0",
    "s": "0x0",
    "v": "0x0"
  }
]

alloc

{
  "0x00000000000000000000000000000000000000f1": {
    "code": "0x736981f674fbaf9d6f293eaffc5855402eb64b8cbd5f5f5f5f5f85610924f260045c600355",
    "storage": {
      "0x0000000000000000000000000000000000000000000000000000000000000002": "0x0000000000000000000000000000000000000000000000000000000000000007"
    },
    "balance": "0x0",
    "nonce": "0x0"
  },
  "0x6981f674fbaf9d6f293eaffc5855402eb64b8cbd": {
    "nonce": "0x1",
    "balance": "0x0",
    "code": "0x600160045d5f5f5f5f5f6006610649f16001600255"
  },
  "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
    "code": "0x",
    "balance": "0xffffffffff",
    "nonce": "0x0"
  }
}

@holiman
Copy link
Contributor

holiman commented Apr 7, 2024

Thanks. Yeah, I had tried that too, but didn't work. Turns out that I had mismatched 1/4 value vs slot on the tload :)

holiman added a commit to holiman/goevmlab that referenced this issue Apr 12, 2024
Testcases for the eels-bugs:

- [x] ethereum/execution-specs#911
- [x] ethereum/execution-specs#914
- [x] ethereum/execution-specs#917

---------

Co-authored-by: Guruprasad Kamath <48196632+gurukamath@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-spec Area: specification C-bug Category: this is a bug, deviation, or other problem.
Projects
None yet
Development

No branches or pull requests

4 participants