fix: update bindings, fix tests#239
fix: update bindings, fix tests#239stephenctw wants to merge 1 commit intofeature/bump-emulator-2from
Conversation
| self.machine, | ||
| dir_cstr.as_ptr(), | ||
| cartesi_machine_sys::CM_SHARING_CONFIG, | ||
| cartesi_machine_sys::CM_SHARING_ALL, |
There was a problem hiding this comment.
tldr; this change is correct.
I asked the machine team about this function.
The answer is that they are future proofing it for lambda.
For now this(CM_SHARING_ALL) option is the only that makes sense.
GCdePaula
left a comment
There was a problem hiding this comment.
Why the "serde default" everywhere? Like, what changed in the new machine version that required us to add this "serde default"? Can you also explain what this default does?
There was a problem hiding this comment.
How will this change behave if we need to use an unreleased machine?
There was a problem hiding this comment.
Do we need these changes?
Apparently the machine now makes a lot of fields/configs optional. There are scenarios that the returned config is not "complete", and the jaon parser will crash of missing fields. So adding this default avoids parser error |
@GCdePaula Could you help taking a second look at the Rust-bindings. They are mostly done by AI agent, which works now, but you might want to make some changes.