Fix rinkeby consensus bug #1094
Conversation
|
CC @holgerd77 |
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Just checked, return value is also right. I have no idea what is going on here. Are we sure that we are on the right hard fork in this block? |
|
Will stop here. This is a very weird situation. I have no idea what's going on 🤔 |
|
Ah, I think I see the problem! The created address as reported by our VM is 0xe962... but the actual created address is 0xa5b2... Does clique have a different way to calculate contract addresses? @holgerd77 |
|
@jochem-brouwer no. The only changed thing in respective to addresses is the COINBASE opcode returning the active signer for clique. |
|
Where does this state root come from? 135fae6d4909a072d77707a797769afc72f6239a72173e0f411c7700af66ac26 Account has nonce 1 when we do the transaction (should be 0) (on above's state root). How come VM does not throw when we run this block? It should see that it cannot execute the transaction; transaction in the block has nonce 0, but the DB has nonce 1, so VM should not execute the transaction, otherwise we can replay transaction on the VM? |
| const trie = new Trie(stateDB) | ||
| const stateManager = new DefaultStateManager({ trie, common }) | ||
| // Ensure we run on the right root | ||
| stateManager.setStateRoot(Buffer.from('135fae6d4909a072d77707a797769afc72f6239a72173e0f411c7700af66ac26', 'hex')) |
There was a problem hiding this comment.
If I change this to:
// Ensure we run on the right root
stateManager.setStateRoot(Buffer.from('0c8c521bd9c5272533db5ec1c9f73a6c727e343005f8a5c61e197d2953c0e165', 'hex'))(This is the state root of block 14181). Then the test passes... (Valid receipt trie, valid root, etc.) There seems to be something going wrong with setting the state root somewhere? It seems to set the state root wrong at some point? My gut says that this might have to do with a bug in clique somewhere? @holgerd77
There was a problem hiding this comment.
I can't follow the argumentation, if you set the state root to the wrong state root (or am I missing something here?) then the test passes, does this say anything (except that it is really the state root that is wrong?)? 🤔 This can still be anything leading to a state root failure.
Please correct me if I am missing the point.
There was a problem hiding this comment.
This 0x0c8c... is the state root of block 14181. I don't know where this original 0x135f... root comes from? I also forgot if I copied some of your code since I cannot find the original (😅 )
There was a problem hiding this comment.
Update: ah, sorry, this is the pre-state root, right? Is this ensured that this is the correct one?
There was a problem hiding this comment.
Yes, it should be; it is the state root of the previous block (which is the state root after processing the block). I took the state root from #1091.
There was a problem hiding this comment.
Ah, I think my state root request for the script from here is likely wrong? But with the corrected state root the tests pass? Why this? 😄 Shouldn't they fail?
There was a problem hiding this comment.
(or is it? I am getting confused, too hungry. 😛 )
There was a problem hiding this comment.
Yes good point - it should fail, since it fails in client. So that implies that client or clique is somehow messing up the state root.
There was a problem hiding this comment.
Also put a comment on your code above.
client: ensure we update VM opcodes when switching
|
Closing here, this one is fixed in #1101. I messed up the rebase - I first rebased on master, then wanted to remove these debug tools commits, but then I'd have to search through a lot of commits. Better to just open a new branch. |
Related: #1090
Pretty dirty PR, but setups a test which setups the right context. What is very strange is that, when comparing traces, the gas used and the stack operations are all good. Except the state root is not correct in the end..?
The only thing I can think of is either (1) the miner is paid an incorrect amount or (2) the deposited code is not right (so there's an issue in memory).