consolidations, withdrawals: return type byte at start of output #25
Conversation
| mul ;; [record_offset, i, count, head_idx, tail_idx] | ||
| mul ;; [offset, i, count, head_idx, tail_idx] | ||
| push 1 ;; [1, offset, i, count, head_idx, tail_idx] | ||
| add ;; [record_offset, i, count, head_idx, tail_idx] |
There was a problem hiding this comment.
I wonder if it is better to add a loop-level item record_offset and just add RECORD_SIZE during each iteration. Feels weird to +1 every calculation.
There was a problem hiding this comment.
Yeah, so that would work as well. Let me know if you prefer this.
There was a problem hiding this comment.
I even thought about turning the whole thing around to keep the offset permanently near the stack top and increment after every store. But then we'd need to find a clean way to hoist up the items from below.
There was a problem hiding this comment.
Eh looking into this more, this is pretty straightforward and understandable implementation. I think we can merge as-is and decide later if it makes sense to optimize it.
There was a problem hiding this comment.
It's less about optimizing and more about correctness. The advantage of a rolling offset is that we only need to add the length of the current item after writing the item. It removes the hard-coded offsets. But I can look into this as a refactoring after this PR is in.
lightclient
force-pushed
the
output-type-prefix
branch
from
eb0fccb to
7371e59
Compare
|
Do we want this as part of Devnet-4? |
Yes, I believe we want requests design to be settled down with its latest version becoming a part of devnet-4 |
4 participants
This is an alternative to #22. It is better to return the request type only once, at the start of return data, to avoid the theoretical possibility where a contract could return multiple different event types.