Neo: Storage.Find duplicate values

[ixje]

The Neo.Storage.Find SYSCALL returns duplicate results causing the invoker to be charged for unnecessary extra Neo.Iterator.Key and Neo.Iterator.Next calls.

Take this contract
```C#
using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Services.Neo;

namespace NeoContract1
{
public class Contract1 : SmartContract
{
public static void Main(int op)
{
if (op == 1)
{
Storage.Put(Storage.CurrentContext, "AA_1", "1");
Storage.Put(Storage.CurrentContext, "AA_2", "2");
Storage.Put(Storage.CurrentContext, "AA_3", "3");
} else if (op == 2)
{
Iterator notify_results(si);
}
else if (op == 3)
{
Storage.Get(Storage.CurrentContext, "AA_1");
Iterator notify_results(si);
}
}

    private static void notify_results(Iterator<string, byte[]> si)
    {
        Runtime.Notify("Starting");
        while (si.Next())
        {
            Runtime.Notify(si.Key);
        }
        Runtime.Notify("Done");
    }
}

}

Invoke the contract once with the contract `op` argument being `1` to persist the contents to the DB. Next invoke the contract with `op` being `2` and `3`.
- `2` gives the expected:  `AA_1`, `AA_2`, `AA_3`,
- `3` however gives:  `AA_1`, `AA_2`, `AA_3`, `AA_1`, `AA_1` 
   The last 2 values should not be there. I believe this is a caching issue.

Take a secondary contract that allows us to call Contract 1 multiple times in a single DB snapshot
```C#
using Neo.SmartContract.Framework;
using Neo.SmartContract.Framework.Services.Neo;
using System;
using System.Numerics;

namespace NeoContract2
{
    public class Contract2 : SmartContract
    {
        [Appcall("f4032cfb5505c05d935db8bd41b608005eca0b00")]
        public static extern void OtherContract(int op);

        public static void Main(int op)
        {
            if (op == 1)
            {
                OtherContract(2);
                OtherContract(2);
            }
            else if (op == 2)
            {
                OtherContract(2);
                OtherContract(3);
            }
            else
            {
                OtherContract(3);
                OtherContract(2);
            }
        }
    }
}

• op is 1 is still correct and produces: AA_1, AA_2, AA_3 followed by another AA_1, AA_2, AA_3
• op is 2 is wrong in the same way it was wrong when calling contract 1 directly. We first get AA_1, AA_2, AA_3 followed by AA_1, AA_2, AA_3, AA_1, AA_1
• op is 3 is where it gets really interesting. Given the previous results you'd expect: AA_1, AA_2, AA_3, AA_1, AA_1 followed by AA_1, AA_2, AA_3. Instead you now get 2 times the sequence AA_1, AA_2, AA_3, AA_1, AA_1

Contract AVM files included in the zip file for easy deploy/reproducing
contracts.zip

[shargon]

Thanks for the report, i will try to find and fix the issue

[vncoelho]

@ixje, Reproduced on NeoCompiler Eco and the error was not found.

Operator 3 still returned:

Notification 0 typeI: Array; typeII: ByteArray; value(hex): 5374617274696e67; value(string): Starting.
Notification 1 typeI: Array; typeII: ByteArray; value(hex): 41415f32; value(string): AA_2.
Notification 2 typeI: Array; typeII: ByteArray; value(hex): 41415f33; value(string): AA_3.
Notification 3 typeI: Array; typeII: ByteArray; value(hex): 41415f31; value(string): AA_1.
Notification 4 typeI: Array; typeII: ByteArray; value(hex): 446f6e65; value(string): Done.

[ixje]

what version did you test against?

[vncoelho]

I am removing the Bug flag from now, @lock9, let's test it before marking as bug, which is a critical thing.
I believe things need double test before setting as bug, which causes a huge impact for those who see the issue.
Thus, let's be careful when using this tag.

I am not saying that this is not a bug, but this is not yet confirmed to be marked as that.

[vncoelho]

@ixje, I used the last version 2.9.2.

[vncoelho]

You can try online if you want: https://neocompiler.io/#!/ecolab/compilers

Please let's us know if there is something strange.
Maybe I did something wrong during the tests. Not sure.

I am just making sure that we do not cause alarms to the community before things are really confirmed.

[ixje]

what is 2.9.2? Why not 2.10.x series?

[vncoelho]

Sorry, version 2.10.2...aehuahea My mistake.

[ixje]

ugh, I thought my network upgraded to 2.10.2, still 2.10.1. let's me check again

[lock9]

@vncoelho what tag do you think we should use when the issue is "on triage"? I added the bug flag because I usually agree with the customer before arguing back (especially when he provides evidence).

I didn't add the critical flag to it, I don't think that "all bugs are critical", in this case, the "bug" would cause the user to pay some extra gas when running the SC, so I believe it is not "a critical bug".

Also:
If the code runs differently in neo-cli versions, with different results, isn't this going to cause a break in network synchronization?
Isn't the gas cost going to be different?

[vncoelho]

No need for extra tag, I am just saying to confirm before tagging.

[lock9]

But all issues need to be tagged, it has been like this since the old days. We have 96 issues open, 95 have tags, why this one is different?
I just didn't use the "discussion" tag, because this doesn't look like a discussion. This looks more like a "support request", I agree that bug may not be the best tag, but we need to tag it.

[ixje]

I should not report issues after an extra long day of debugging. Made a rookie mistake by looking at neo-python's interpretation 😅

[vncoelho]

No worry, @ixje...You are at home.
I imagined it was something about neopy interpretation and for saving @shargon's precious efforts I took some time to check it out.

Lets keep investigating everything. We are all contributing with differents things.

[vncoelho]

Ricarrrdo, @lock9, here is our flag! :dagger: