Add support for state_header access in the SDK - #205
Conversation
| /// slot index is `ts - 1` and the step name is `step_{ts-1}`. | ||
| pub(crate) fn chain_step_at(ts: usize, chain_max_ts: usize) -> Option<usize> { | ||
| (chain_packed(chain_max_ts) && ts > 0 && ts < chain_max_ts).then_some(ts - 1) | ||
| (chain_packed(chain_max_ts) && ts > 0 && ts < chain_max_ts).then(|| ts - 1) |
There was a problem hiding this comment.
There was an underflow here (which was detected in debug mode).
On underflow this results in None, but in debug an underflow panics. By using a closure the subtraction only happens when the check passes.
8150748 to
dffc01c
Compare
dffc01c to
b46f259
Compare
1fccba6 to
2c61de0
Compare
|
what does |
|
FYI - confirmed e2e objects are produced and synchronizer runs fine Some docs are outdated and the GUI rendering is broken cuz of the new object structure but I updated those here #206 |
This is inspired by the read/write/execute flags of files display in Unix. So
|
robknight
left a comment
robknight
left a comment
There was a problem hiding this comment.
Seems good overall, just some questions about how io is handled and some outdated comments which need tidying up.
| let res = executor.action("Tick", vec![ticker0]).unwrap(); | ||
| let ticker1_tx = res.tx.clone(); | ||
| let [_ticker1] = res.objs(); | ||
| apply_tx(&mut state, &ticker1_tx); |
There was a problem hiding this comment.
There are no asserts in this test. Are we just testing for panics?
There was a problem hiding this comment.
The intention here is to run the action and make sure it succeeds and the statements are built satisfied. So yeah, we're testing for panics because we're unrwapping the result of executor.action. Do you think some asserts would be useful here?
There was a problem hiding this comment.
No, I just wanted to check the rationale.
|
I've merged #206 into this PR's branch, which fixed most (if not all) of the documentation issues @robknight mentioned. |
robknight
left a comment
robknight
left a comment
There was a problem hiding this comment.
lgtm, the merge addresses the issues I raised.
4 participants
Resolve #201
I had to merge the In/Out records into a single IO one because even after the splitter did its thing, there were predicates that had 9 wildcards (our max is 8).
I believe this change doesn't introduce any performance regression and gets rid of one wildcard in some occasions.
Note for the reviewer: I haven't yet tested the correct behavior of the synchronizer after this PR nor an end 2 end flow because I don't have access to my everyday machine and I did this on 8GB of RAM 🫠