Algod: Simulate endpoint scratch-change exposure #5563
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5563 +/- ##
==========================================
+ Coverage 55.80% 55.82% +0.01%
==========================================
Files 446 446
Lines 63417 63461 +44
==========================================
+ Hits 35389 35426 +37
- Misses 25663 25665 +2
- Partials 2365 2370 +5
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
d59d8a2 to
1217aed
Compare
d3d5399 to
3f9650a
Compare
data/transactions/logic/opcodes.go
Outdated
| // and if so, it returns scratch slot id, previous value, new value to be written; | ||
| // otherwise, it indicates the current opcode is not a scratch slot chagne. | ||
| func (cx *EvalContext) CurrentScratchChange() (scratchSlot uint64, newValue basics.TealValue, isScratchChange bool) { | ||
| currentOpcodeName := opsByOpcode[cx.version][cx.program[cx.pc]].Name |
There was a problem hiding this comment.
I think it does make sense to make these operations easier. Ideally this code would be placed in the simulation package.
Passing in the OpSpec to Before/AfterOpcode would definitely help. I can also imagine having an EvalContext.GetOpSpec(pc int) function as an alternative. Right now I have no preference between the two.
You're right in that we'd also need to expose EvalContext.program, since this code needs to figure out immediate args as well.
9adc169 to
697684c
Compare
1130aff to
fe63446
Compare
| if len(scratchChanges) == 0 { | ||
| return nil | ||
| } | ||
| modelSC := make([]model.ScratchChange, len(scratchChanges)) |
There was a problem hiding this comment.
its so weird we have to write these
data/transactions/logic/opcodes.go
Outdated
| // and if so, it returns scratch slot id, previous value, new value to be written; | ||
| // otherwise, it indicates the current opcode is not a scratch slot chagne. | ||
| func (cx *EvalContext) CurrentScratchChange() (scratchSlot uint64, newValue basics.TealValue, isScratchChange bool) { | ||
| currentOpcodeName := opsByOpcode[cx.version][cx.program[cx.pc]].Name |
There was a problem hiding this comment.
how about Before/After opcode getting the OpSpec and the OpDetail extended to include a SlotsModified logic function which returns nothing except for store/stores, where it runs this logic.
There was a problem hiding this comment.
Feel free to make a change described in this thread if you want to, but I view it more as a nit-level issue: #5563 (comment)
eee45a4 to
0d1276b
Compare
0d1276b to
ec29772
Compare
ec29772 to
72d2f3b
Compare
Summary
This PR implements #5013 : we can now request simulation endpoint for writes into scratch slots, as part of the execution trace response.
Test Plan