Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

APPEND can return NULL? #3701

Open
Slava0135 opened this issue Nov 25, 2024 · 3 comments
Open

APPEND can return NULL? #3701

Slava0135 opened this issue Nov 25, 2024 · 3 comments
Labels
enhancement Improving existing functionality I3 Minimal impact S4 Routine U3 Regular vm VM tasks/bugs/issues
Milestone

Comments

@Slava0135
Copy link
Contributor

Have been fuzzing VM again and found this.

Problem

NEO-GO-VM > loadbase64 EMNKSs8=
READY: loaded 5 instructions
NEO-GO-VM 0 > ops
INDEX    OPCODE      PARAMETER
0        PUSH0           <<
1        NEWARRAY    
2        DUP         
3        DUP         
4        APPEND      

NEO-GO-VM 0 > run
[
    null
]

But i am not sure if this is intended because C# implementation throws exception when calling ToJson:

Unhandled exception. System.InvalidOperationException: Circular reference.
   at Neo.VM.Helper.ToJson(StackItem item, HashSet`1 context, Int32& maxSize)
   at Neo.VM.Helper.ToJson(StackItem item, HashSet`1 context, Int32& maxSize)
...

There is no such case in tests, so i am asking here.

@Slava0135 Slava0135 added question Further information is requested U3 Regular labels Nov 25, 2024
@roman-khimov
Copy link
Member

I think that you don't have a real null here in fact, likely that's just the way it's stubbed out (because it can't be JSONized/represented). This can be checked by some lower-level UT. But I'm surprised by the C# exception. Related to neo-project/neo#3581, I think that after it happens the behavior will just be the same as in NeoGo.

@roman-khimov roman-khimov added the vm VM tasks/bugs/issues label Nov 25, 2024
@roman-khimov
Copy link
Member

throws exception when calling ToJson

Ah. Sorry, haven't noticed immediately. I think that the VM behavior is the same then and will remain the same. But then we have different JSONization choices at different places. RPC call would give a different output, for example. And C# exception is fine, it's also handled in various places dealing with JSON.

@roman-khimov roman-khimov added I3 Minimal impact enhancement Improving existing functionality S4 Routine and removed question Further information is requested labels Nov 29, 2024
@roman-khimov roman-khimov added this to the v0.108.0 milestone Nov 29, 2024
@roman-khimov
Copy link
Member

We can improve the output here to show some error in place of a particular wrong element.

@AnnaShaleva AnnaShaleva modified the milestones: v0.107.2, v0.108.0 Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality I3 Minimal impact S4 Routine U3 Regular vm VM tasks/bugs/issues
Projects
None yet
Development

No branches or pull requests

3 participants