Compiling transaction fails with multiple notes

[igamigo]

When testing consuming multiple notes from the client, I'm getting this error:

transaction executor error: CompileTransactionFiled(CompileNoteScriptFailed(DuplicateProcName("add_note_assets_to_account", "#exec"))) 

The same code works for consuming one single note each time, the issue occurs only when having more than one. I'm not sure if this is a bug or a problem with the client code (I couldn't find examples of transactions having multiple input notes), but @bobbinth suggested opening an issue here.

[bobbinth]

This is likely due to a collision in the Assembler's cache when we try to compile two executable programs in the same context. So, it is likely that the bug is in Miden VM rather than miden base. @Overcastan may be able to help with this.

[Fumuran]

@igamigo Could you please provide the code of the failing test?

[igamigo]

Here's the function that creates the transaction that compiles the script and attempts to consume multiple notes. Notice lines 256-264, where it is decided to use either one or multiple notes; first branch works correctly but the else clause does not. Unfortunately it's not very simple to test right now but I will try to provide instructions or a test that runs this code and repros the issue tomorrow.

[igamigo]

Having some trouble creating a unit test for this, I'm getting a PhantomCallsNotAllowed error for the test I was writing. Guessing I'm mocking some inputs incorrectly.

[Fumuran]

@igamigo I think I understood what's happening, there is no need for the example test. Bobbin is right, the problem occurs when we try to compile two programs in the same context.
During the nodes consumption you're compiling the transactions, which internally contains the compilation of the note scripts (P2ID or P2IDR, I guess). Both of these scripts contain the add_note_assets_to_account procedure, and that's exactly what causes the error: we don't allow usage of functions with the same names in one context now. If I remove this check the local procedures should still run without problems, but I'm not sure about exported and reexported — let me check this.
Anyway now I see that this check should be reworked, at least we need to print a warning if we compile two procedures with the same names. That shouldn't be a complex issue to fix, I hope I will open a related PR tomorrow

[Dominik1999]

@Overcastan are you still on this topic?

[Fumuran]

Yes, problem is in the VM, I hope I'll create a PR on the weekend.

[Fumuran]

@igamigo I checked the VM and came to the conclusion that just a plane change from error to log message should be fine. Although I didn't manage to create a proper test for the case with two programs compiled in one context. Could you check your test with my VM dev branch andrew-duplicate-proc-fix as a dependency? Instead of error now it should print a warning: duplicate proc name '{}' in module {}.

[igamigo]

Will test and report back, thanks

[igamigo]

Yes, this works well (took a a bit to test due to the dependency chain of client -> base- > VM). Thanks!

[Fumuran]

Sorry, changing dependencies is troublesome indeed, I felt this when I was updating the repositories to work with new version of Winterfell. I'll create a PR with this change today

[Dominik1999]

@Overcastan are you still working on this issue?

[Fumuran]

@Dominik1999 Probably we should put this issue on hold, since it should become a non-issue when @bitwalker merge the changes in assembler.

[Dominik1999]

@Overcastan are the changes in the assembler done here 0xPolygonMiden/miden-vm#1277 enough? Or is another PR outstanding?

[Fumuran]

@igamigo Can you please check is this error still occurring with a new version of assembler?

[Dominik1999]

It is still blocked

[bobbinth]

Closed by #837.