AVM: Only update the bytec disassembleState for bytecblock opcodes

[nullun]

During disassembly, if a contract includes a pushbytess opcode after the bytecblock, it will no longer overwrite the bytec disassembleState. This resulted in incorrect or missing comments for disassembled bytec opcodes.

Of course, there is still the issue of someone using bytecblock multiple times, however that's easier to identify. This threw me for a while.

Test program:

cat test.teal                                                  
#pragma version 10
bytecblock 0x6869 0x414243 0x74657374 0x666f7572 0x6c617374
bytec_0
bytec_1
bytec_2
bytec_3
bytec 4
pushbytess 0x6576696c
bytec_0
bytec_1
bytec_2
bytec_3
bytec 4

Before:

goal clerk compile test.teal -o - | goal clerk compile -D - 
#pragma version 10
bytecblock 0x6869 0x414243 0x74657374 0x666f7572 0x6c617374
bytec_0 // "hi"
bytec_1 // "ABC"
bytec_2 // "test"
bytec_3 // "four"
bytec 4 // "last"
pushbytess 0x6576696c
bytec_0 // "evil"
bytec_1
bytec_2
bytec_3
bytec 4

After:

goal clerk compile test.teal -o - | goal clerk compile -D - 
#pragma version 10
bytecblock 0x6869 0x414243 0x74657374 0x666f7572 0x6c617374
bytec_0 // "hi"
bytec_1 // "ABC"
bytec_2 // "test"
bytec_3 // "four"
bytec 4 // "last"
pushbytess 0x6576696c
bytec_0 // "hi"
bytec_1 // "ABC"
bytec_2 // "test"
bytec_3 // "four"
bytec 4 // "last"

[jannotti]

I assume we have the same problem for intc, with pushints?

[nullun]

I assume we have the same problem for intc, with pushints?

Good thinking! Let me take a look and add the change for that too.

[nullun]

As I did before, here was my local test.

Test program:

cat test.teal 
#pragma version 10
intcblock 0 1 2 3 4 5
intc_0
intc_1
intc_2
intc_3
intc 4
pushints 6
intc_0
intc_1
intc_2
intc_3
intc 4

Before:

goal clerk compile test.teal -o - | goal clerk compile -D - 
#pragma version 10
intcblock 0 1 2 3 4 5
intc_0 // 0
intc_1 // 1
intc_2 // 2
intc_3 // 3
intc 4 // 4
pushints 6
intc_0 // 6
intc_1
intc_2
intc_3
intc 4

After:

goal clerk compile test.teal -o - | goal clerk compile -D -
#pragma version 10
intcblock 0 1 2 3 4 5
intc_0 // 0
intc_1 // 1
intc_2 // 2
intc_3 // 3
intc 4 // 4
pushints 6
intc_0 // 0
intc_1 // 1
intc_2 // 2
intc_3 // 3
intc 4 // 4

[jannotti]

Love it. I'll wrangle another review.

[cce]

LGTM — btw, IMO bug fix PRs like this should include tests, so I took your before/after examples and turned them into one

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.25%. Comparing base (0d10b24) to head (fe97864).
Report is 13 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6154      +/-   ##
==========================================
- Coverage   56.49%   56.25%   -0.25%     
==========================================
  Files         499      494       -5     
  Lines       70029    69956      -73     
==========================================
- Hits        39566    39354     -212     
- Misses      27796    27935     +139     
  Partials     2667     2667              

Flag	Coverage Δ
	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.