algorandfoundation / ARCs

Algorand Requests for Comments
Creative Commons Zero v1.0 Universal
127 stars 110 forks source link

ARC28 events spec does not specify how to encode arguments #252

Open joe-p opened 9 months ago

joe-p commented 9 months ago

ARC28 is ambiguous when it comes to how arguments to an event should be encoded. It's not clear if arguments should be encoded separately and then concatenated or if they should be encoded as a tuple. The example shows (uint64,uint64), but the encoding for this is the same if they are concatenated or if it's encoded as a tuple.

For dynamic types, it should be made clear (via an additional ARC since ARC28 is final) what the expected encoding is. I personally think it makes the most sense to encode the arguments as a tuple so they can be easily parsed with existing ABI tooling.

nullun commented 7 months ago

I had thought it followed the exact same ARC4 method encoding rules, minus the return value. In fact, I had assumed a cheeky TEAL developer could even use the same pseudo-op method in their TEAL to generate the Event Signature that matches the ARC.

#pragma version 9

method "Swapped(uint64,uint64)"
int 42
itob
int 100
itob
concat
concat
log

int 1
$ goal clerk compile event.teal -o - 2>/dev/null | goal clerk compile -D -
#pragma version 9
pushbytes 0x1ccbd925 // 0x1ccbd925
pushint 42
itob
pushint 100
itob
concat
concat
log
pushint 1
      "logs": [
        "HMvZJQAAAAAAAAAqAAAAAAAAAGQ="
      ],

Whilst a method signature should include a return value, if you leave it out you are only presented with a warning during compiling. I'm sure I'll get some funny looks for this. :smile:

joe-p commented 7 months ago

Yeah the method signature is the same, but I'm talking about the actual values.

For example, if you had swapped(uint8[],uint8[]) should you encode the values as a tuple (uint8[],uint8[]) or encode the arguments separately uint8[] and uint8[] and concat them (without the head elements of the tuple).

I think the obvious answer is to encode as a tuple, but it should be specified in the ARC (I would've sworn I PR'd this already but I guess not)

CASABECI commented 3 months ago

ARC28 is ambiguous when it comes to how arguments to an event should be encoded. It's not clear if arguments should be encoded separately and then concatenated or if they should be encoded as a tuple. The example shows (uint64,uint64), but the encoding for this is the same if they are concatenated or if it's encoded as a tuple.

For dynamic types, it should be made clear (via an additional ARC since ARC28 is final) what the expected encoding is. I personally think it makes the most sense to encode the arguments as a tuple so they can be easily parsed with existing ABI tooling.

nullun commented 3 months ago

Closed by #291