List parameters

[NeoDashboard]

Hello,
I was wondering if there is a reason behind the choice of implementation of list parameters using APPEND/DUP/NEWARRAY0 opcodes as It seems to not be necessary?

[ixje]

I'm not sure I understand what you're referring to. Can you elaborate or point to the code where this is happening?

[ixje]

You mean this?

neo-mamba/neo3/vm.py

Lines 333 to 339 in d7e2c02

	elif isinstance(value, Sequence):
	self.emit(OpCode.NEWARRAY0)
	for v in value:
	self.emit(OpCode.DUP)
	self.emit_push(v)
	self.emit(OpCode.APPEND)
	return self

I'm open to suggestions on how you think we can improve it

[NeoDashboard]

Sorry I'll try with a screenshot :) As you can see using last version of neo-mamba when passing a list as parameter It's using some opcodes I put in first message in the transaction script. But if I build It myself without those append/dup... It's still working so I'm wondering if there is something I'm missing?

[ixje]

Can you share an example how you're building your array?

[NeoDashboard]

I was using this on my side previously

class ScriptBuilderWithListSupport(vm.ScriptBuilder):
    def emit_push_list(self, items: list):
        for item in reversed(items):
            if type(item) is list:
                self.emit_push_list(item)
                continue
            self.emit_push(item)
        self.emit_push(len(items))
        self.emit(vm.OpCode.PACK)

    def emit_dynamic_call_with_args(self, script_hash, operation: str, args) -> None:
        self.emit_push_list(args)
        self.emit_push(0xF)
        self.emit_push(operation)
        self.emit_push(script_hash.to_array())
        self.emit_syscall(vm._syscall_name_to_int("System.Contract.Call"))

[NeoDashboard]

This?

    def emit_push_list(self, items: list):
        for item in reversed(items):
            if type(item) is list:
                self.emit_push_list(item)
                continue
            self.emit_push(item)
        self.emit_push(len(items))
        self.emit(vm.OpCode.PACK)

[ixje]

It's been a while so I'm going to let it sink in and try to remember why I took this approach. If I can't come up with a good reason then we can change it to use PACK (should also be cheaper)

[NeoDashboard]

Yeap cheaper and more beautiful that's why I opened an issue :D

[ixje]

It's been a while so I'm going to let it sink in and try to remember why I took this approach. If I can't come up with a good reason then we can change it to use PACK (should also be cheaper)

I can't think of anything at the moment, so open to accept a PR if you'd like

[NeoDashboard]

Something like this? #255

[Devel484]

It's been a while so I'm going to let it sink in and try to remember why I took this approach. If I can't come up with a good reason then we can change it to use PACK (should also be cheaper)

I can't think of anything at the moment, so open to accept a PR if you'd like

The previous implementation with DUP and APPEND was used to invert the order of the emitted items . The solution from @NeoDashboard emits the items directly in the correct (inversed) order and is cheaper.

[ixje]

Fixed in #256