Giulio

Results 43 comments of Giulio

> I tentatively agree with you about you thinking about `LogicSigMaxSize()`. There is no explicit constant for a maximum argument size, so an argument can indeed by the whole 16kb...

OK, wasn't too bad, studying [algorand/msgp](https://github.com/algorand/msgp) I found an handy `maxtotalbytes` directive which I used to fix the max size of the argument array: ``` diff @@ -39,7 +39,7 @@...

You made me realize that indeed we need to decide which is the max limit for a logicsig arg. Before we had a combined cap for Logic + Args of...

> Yes, we ought to allow 4096 for individual args. I suppose it can be checked in `transactions/data/logic/eval.go` near here: > > ``` > if cx.txn.Lsig.Args != nil && len(cx.txn.Lsig.Args)...

> After further discussion, we're confident that we can make changes to the max size tests so it doesn't fail so easily, without issue. I'll make those changes in a...

I like it, i had to make two changes to pass all tests and make all checks green. The first change was in `TestMaxSizesCorrect`: ``` diff node/node_test.go @@ -824,10 +824,10...

> Maybe we _do_ have such a test, but it is being run on a single txn group, so it retains the same behaviour as before. Could you look around...

I think this PR is good to go now. Let me know if you prefer that I move the integration [test](https://github.com/algorand/go-algorand/pull/6057/files#diff-8018e89516809642bdc6478498569b4f27c027ae52bec137a61e2e6812e18ac7) I added to a unit test instead

> There are two small linter errors that should be fixed. They just appeared because the linter run needed approval to run. Done, I could have run `make lint` locally...

All looks good to me now; codecov shows some lines not covered in `cmd/goal/clerk.go` but I suppose that's ok; let me know otherwise