feat: impl contract calls #26

0xrusowsky · 2024-12-14T01:29:47Z

this PR brings x-contract calls to r55.

interface macro

the most opinionated design choice that i made is to create a macro (inspired by the contract macro) that based on an interface (trait) generates the necessary syntactic sugar to call a contract, handling abi en/decoding and the risc-v runtime call.

as u can see in erc20x the user simply defines an interface as they would do in solidity
https://github.com/0xrusowsky/r55/blob/110cc5addd481368723eb0b992f081231c2b8d7d/erc20x/src/lib.rs#L17-L20

and then they can use it by initializing with the address of the target contract:
https://github.com/0xrusowsky/r55/blob/110cc5addd481368723eb0b992f081231c2b8d7d/erc20x/src/lib.rs#L25-L29

i wanted to leverage rust's type system, so the macro forces all methods of the interface to return options (in the future it would be cool to support custom errors). As u can see in the example above, unlike with solidity, we force devs to handle potential call errors.

r55 execution

after i had the interface implementation, i had to integrate the calls with revm.
in this case i've had to touch code that was written by others beforehand, so i'll try to justify why i changed some of the stuff that had been previously done >> the main reason is that frames (how revm handles the execution context of each call) hadn't been fully taken into account.

relevant changes that i made:

r55 doesn't handle "evm gas" anymore, as the cumulated amounts were lost between calls. Instead, the gas computation is delegated to the revm interpreter >> i implemented the syscall_gas! macro (maybe not the best name), which allows us to simply worry about the riscv gas. Finally, since i was touching gas, i also moved away from magic numbers and started using constants.
r55 totally ignores revm shared memory (as it uses its on DRAM). The main change here was adding memory management in the handle register. In this case, when the interpreter action is a result (not the last one), we need to write the output to the memory return data destiny that the parent frame is expecting to read from.
in the frame call stack, rather than always getting the first, we had to get the last one, so that it acts like a stack (lifo)

logging and tests

in order to make it work, i had to debug a lot, so i ended up having prints everywhere lol
however, i thought that some of those where quite useful to keep, specially while there is still stuff to implement. Because of that i moved from env_logger to tracing, and added a bunch of debug logs (that won't run by default, but which are really useful when developing).

you can see the debug logs by running:

RUST_LOG=debug cargo test --package r55 --test e2e -- erc20 --exact

PS: i have format-on-save by default, so sorry for that, i know it's annoying.

gakonst

concept ack

gakonst · 2024-12-14T01:54:43Z

r55/src/exec.rs

+            if !is_last {
+                if let Some(Some(parent)) = call_stack.borrow_mut().last_mut() {
+                    if let Some(return_range) = &parent.returned_data_destiny {
+                        if let InterpreterAction::Return { result: res } = &mut result {
+                            // get allocated memory slice
+                            let return_memory = parent
+                                .emu
+                                .cpu
+                                .bus
+                                .get_dram_slice(return_range.clone())
+                                .expect("unable to get memory from return range");
+
+                            debug!("- Return data: {:?}", res.output);
+                            debug!("- Memory range: {:?}", return_range);
+                            debug!("- Memory size: {}", return_memory.len());
+
+                            // write return data to parent's memory
+                            if res.output.len() == return_memory.len() {
+                                return_memory.copy_from_slice(&res.output);
+                            } else {
+                                warn!("Unexpected return data size!");
+                            }
+                        }
+                    }


can u unpack a bit what's going on here

that's what i was trying to explain in the PR description regarding r55 memory.

my understanding is that in revm we have a concept of shared memory, whereas in this case we have DRAM serving the same purpose (hence why we don't need MLOAD and MSTORE syscalls).
since each frame (call) uses an independent riscv context with it's own emulator instance, we need to keep track of the expected return data range location in DRAM (RVEmu.returned_data_destiny).

when the parent [FRAME-0] executes a CALL, we initialize + cache the memory range:
https://github.com/0xrusowsky/r55/blob/7e8eebd29ce061aea541fe33707b0d00c7c0900f/r55/src/exec.rs#L350-#L356

the child [FRAME-1] computes the output of the call, but is unaware of the expected return data destiny of the parent. Because of that i thought that the only place where i could "move data between frames" was in fn handle_register(), cause that's where we manipulate the call stack >> the line where u added the comment is where we have access to both the returned data + the parent context at the same time.

posting a simplified version of the debug logs to help understand the explanation above:

// emited in `handle_register` (call stack) === [FRAME-0] Contract: 0x114c28ea2CDe99e41D2566B1CfAA64a84f564caf =========== // emited in `execute_riscv` (risc-v emu) Starting RISC-V execution: PC: 0x80300000 Return data dst: None [Syscall::call - 0xf1] > Return data will be written to: 2150651684..2150651716 // emited in `handle_register` (call stack) === [FRAME-1] Contract: 0xf6a171f57ACAc30C292e223eA8aDBb28abD3E14d =========== // emited in `execute_riscv` (risc-v emu) Starting RISC-V execution: PC: 0x80300000 Return data dst: None [Syscall::return - 0xf3] // emited in `handle_register` (call stack) >>> this is the step where i copy the output data to the expected location by the parent frame === [FRAME-1] Ouput: RETURN - Return data: 0x000000000000000000000000000000000000000000000000000000000000002a - Memory range: 2150651684..2150651716 - Memory size: 32 // emited in `handle_register` (call stack) === [FRAME-0] Contract: 0x114c28ea2CDe99e41D2566B1CfAA64a84f564caf =========== // emited in `execute_riscv` (risc-v emu) Resuming RISC-V execution: PC: 0x80300994 Return data dst: Some( 2150651684..2150651716, ) Loaded return data: 0x000000000000000000000000000000000000000000000000000000000000002 // emited in `handle_register` (call stack) === [FRAME-0] Ouput: RETURN (last)

assuming that what i explained above makes sense... git blame was pointing to Dragan as the editor of most of the handle_register() code, so maybe u also want him to validate that my proposed approach is sound

based on @leonardoalt comments it looks like my implementation only works with RISCV contracts, as we do need to use shared memory when interacting with EVM contracts

gakonst · 2024-12-14T01:55:20Z

r55/src/exec.rs

-                            });
-                        }
+                        // RETURN logs the gas of the whole risc-v instruction set
+                        syscall_gas!(interpreter, r55_gas);


seems fine right?

when testing i made sure that i got the exact same gas costs as the original implementation.
as mentioned above, this approach fixes the scenario where there are multiple calls (execution contexts)

r55/src/exec.rs

leonardoalt

This is awesome!

Left a few small comments.
Also can you remove the rvemu.* files (we should maybe add them to gitignore).

r55/src/test_utils.rs

r55/src/exec.rs

leonardoalt · 2024-12-14T20:47:29Z

r55/src/exec.rs

                                scheme: CallScheme::Call,
                                is_static: false,
                                is_eof: false,
-                                return_memory_offset: 0..ret_size as usize,
+                                return_memory_offset: 0..0, // handled with `returned_data_destiny`


I wonder if we need this when a RISCV contract calls an EVM contract

r55/src/exec.rs

leonardoalt · 2024-12-14T20:49:34Z

r55/src/exec.rs

+                            debug!("- Memory range: {:?}", return_range);
+                            debug!("- Memory size: {}", return_memory.len());
+
+                            // write return data to parent's memory


This only supports the case where a RISCV contract calls a RISCV contract, right?

yeah, i totally missed that

contract-derive/src/lib.rs

0xrusowsky added 6 commits December 13, 2024 14:41

feat: impl x-contract calls

e2aca56

feat: improve logs

f87940a

fix: info as default log level

267b063

fix: readme

f027af5

chore: split into erc20 + erc20x

110cc5a

chore: housekeeping

7e8eebd

gakonst reviewed Dec 14, 2024

View reviewed changes

chore: housekeeping

1713732

leonardoalt reviewed Dec 14, 2024

View reviewed changes

style: incorporate feedback

a0de7f2

leonardoalt previously approved these changes Dec 16, 2024

View reviewed changes

style: clippy

3b38658

0xrusowsky dismissed leonardoalt’s stale review via 3b38658 December 16, 2024 22:33

leonardoalt approved these changes Dec 17, 2024

View reviewed changes

leonardoalt merged commit 8714771 into r55-eth:main Dec 17, 2024
13 checks passed

0xrusowsky mentioned this pull request Jan 20, 2025

feat: support evm contract calls #34

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: impl contract calls #26

feat: impl contract calls #26

0xrusowsky commented Dec 14, 2024 •

edited

Loading

gakonst left a comment

gakonst Dec 14, 2024

0xrusowsky Dec 14, 2024 •

edited

Loading

0xrusowsky Dec 14, 2024

0xrusowsky Dec 14, 2024

gakonst Dec 14, 2024

0xrusowsky Dec 14, 2024

leonardoalt left a comment

leonardoalt Dec 14, 2024

leonardoalt Dec 14, 2024

0xrusowsky Dec 14, 2024

feat: impl contract calls #26

feat: impl contract calls #26

Conversation

0xrusowsky commented Dec 14, 2024 • edited Loading

interface macro

r55 execution

logging and tests

gakonst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xrusowsky Dec 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonardoalt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xrusowsky commented Dec 14, 2024 •

edited

Loading

0xrusowsky Dec 14, 2024 •

edited

Loading