baking-bad / pytezos

🐍 Python SDK for Tezos | Michelson VM in Python
https://pytezos.org
MIT License
111 stars 36 forks source link

Fix interpreter UNPAIR N to actually count the amount of pairs to unpair #329

Closed konchunas closed 1 year ago

konchunas commented 1 year ago

I had an UNPAIR 8 operation on the following pair:

((((((((5 * 10) * (1 * KT1PWx…itn)) * ((0 * KT1PWx…itn) * 0)) * ((0 * 0) * (<-1> * 1))) * (0 * -1048575)) * (((0 * 0) * <-2>) * (0 * <-3>))) * (((1 * <-4>) * (<-5> * 1208925819614629174706176)) * <-6>)) * (-1048575 * (1048575 * ((100000 * 100000) * (KT1BEq…DLi * (KT1BEq…DLi * (10000 * (0 * 0))))))))

Current Pytezos implementation takes every second element of a pair and unpairs it, regardless of N. This approach failed for me on assert with expected 8 leaves, got 9 leaves. It happens because the very last item (0 * 0) is also a pair and it gets unpaired. I've made this example implementation which actually unpairs 8 elements instead.

Marking this pull request as a draft, cause it might be better to add a count to iter_comb function.

m-kus commented 1 year ago

Hey! Thanks for the fix, indeed I haven't considered this case. It's ok, better leave iter_comb as it is.

Seems that integration tests are falling due to the node, can I please ask you to replace https://github.com/baking-bad/pytezos/blob/ec5482ef02f9722bdea50d99cedd0bb01452e60f/src/pytezos/context/mixin.py#L32 with 'mainnet': ['https://rpc.tzkt.io/mainnet'], ?

konchunas commented 1 year ago

Integration tests passed without problems now

m-kus commented 1 year ago

Thank you! Merging