Conversation
d8c9171 to
4347b17
Compare
33e9bc4 to
6e5c5fa
Compare
| } | ||
|
|
||
| // TestSimulator starts the in-memory L1 simulator. | ||
| func TestSimulator() func() { |
There was a problem hiding this comment.
I am curious the reason to return function here
There was a problem hiding this comment.
The initial idea was for clean up, so it behaves like the other functions. We don't need cleanup but at least should remain the signature.
| // (per-byte for new/mutated objects) - storage_rebate. Reproducing this | ||
| // requires executing Move bytecode, which the simulator doesn't do. | ||
| // TODO: Marker for adding Gas cost calculation, but this technically requires actual execution of contracts. | ||
| gasCost := uint64(0) |
There was a problem hiding this comment.
Should we set a small const value as testing gas cost?
There was a problem hiding this comment.
That would be the best case scenario. I spent a day to improve upon this. Without a proper Move processing calculation + Gas calculation for handling the raw PTB any change in here either breaks some tests or other kinds of test.
This can be improved probably, but thats a big can of worms.
dessaya
left a comment
There was a problem hiding this comment.
🎉
Too big to do a 100% detailed review. I focused on the test_simulator code and it looks just fine 👏
Description of change
This PR removed the legacy JSON-RPC types, conversion and mapper functions.
It also adds an experimental fake L1 simulator, that can be plugged into all tests.