js-algorand-sdk icon indicating copy to clipboard operation
js-algorand-sdk copied to clipboard

ABI decoder does not allow zero-length arrays

Open jeapostrophe opened this issue 4 years ago • 3 comments

https://github.com/algorand/js-algorand-sdk/blob/develop/src/abi/abi_type.ts#L31

but

https://github.com/algorandfoundation/ARCs/blob/main/ARCs/arc-0004.md#types --- "<type>[<N>]: A fixed-length array of length N, where N >= 0. type can be any other type."

([1-9][\d]*) should just be [\d]+

jeapostrophe avatar Jan 14 '22 20:01 jeapostrophe

We can't use [\d]+ with the code as is, since it would make things like uint64[0000000001] appear as a valid type. And we don't want this because method signatures need to have consistent hashes -- the ARC should probably clarify this.

But we could modify the regex in another way to support a single 0. Though it's worth noting this type is essentially a noop since you can't store anything in a 0 length static array.

jasonpaulos avatar Jan 14 '22 23:01 jasonpaulos

Just made this: https://github.com/fabrice102/ARCs/pull/2

jasonpaulos avatar Jan 14 '22 23:01 jasonpaulos

Good point!

https://en.wikipedia.org/wiki/Robustness_principle --- I think you should accept uint64[00001] but never generate it =)

jeapostrophe avatar Jan 14 '22 23:01 jeapostrophe

This is still not fixed. You should change the regexp:

const staticArrayRegexp = /^([a-z\d[\](),]+)\[(0|[1-9][\d]*)]$/;

jeapostrophe avatar Nov 15 '22 16:11 jeapostrophe

need to look at the other SDKs as well, probably same issue & fix.

algoanne avatar Nov 17 '22 21:11 algoanne

Thank you! We should also reference #698

jeapostrophe avatar Nov 29 '22 16:11 jeapostrophe

oops github is acting smart here and closed the ticket 😅

ahangsu avatar Nov 29 '22 17:11 ahangsu