feat(sozo): add --stats flag to output stats of the build artifacts

[fabrobles92]

Usage: sozo build --stats
Included the option of --stats for when we do a sozo build; the command will output the number of felts in a contract_sierra.json and also the size of the file..

Closes DOJ-286
Closes #1081

[kariy]

Thanks for the contribution. I have some feedbacks.

I think we can be more conservative with how the stats are being displayed. Requesting for the stats should imo just output the raw stats independent of any limitations imposed by the networks.

And also having to read from a file just for two fields is a bit too excessive imo. And i don't see how --stats-limits flag would be used much currently as no other Starknet-based networks exist that are imposing these limits.

This is in consideration that only a handful of users will deploy to the mainnet/testnet at the moment.

[glihm]

I think we can be more conservative with how the stats are being displayed. Requesting for the stats should imo just output the raw stats independent of any limitations imposed by the networks.

Agree on the more conservative display. About the % I guess this was the original goal of the issue, but we may drop the limitation display and only have a warning based on this limitation?

And also having to read from a file just for two fields is a bit too excessive imo. And i don't see how --stats-limits flag would be used much currently as no other Starknet-based networks exist that are imposing these limits.

I suggested the file into the discord thinking that we may have more, and to avoid having one flag for each limit. But if from the previous point we go without limits being displayed, we could remove it.

In general having the limits could be interesting to get an idea of the current size of the contracts in projection for a deployment on Starknet network (which was a track during the game-jam).
However as you mentioned, we should have more L3 use-cases, where limits have no much importance, that's for sure.

[fabrobles92]

Hi @glihm and @kariy

Some thoughts:

-The stats are always shown no matter if they pass or not the limits. (The 3 possible messages are just informative only for awareness to comply maintaining the starknet network stable and optimized).

-If they pass the limitations this will not prevent the contracts from being built.

-According to some conversations we had, the first iteration consisted only of showing the number of felts by contract and the actual size of the file, in future we might include # Cairo steps per contract, Gas consumption, model size, etc. Thus the creation of the .json constants file. I was just preparing the land for future adds :D

In any case if the limits is not a go. I can delete the 3 messages and only show number of felts by contract and size file.

Please let me know so I can make the modifications :D

Thanks in advance!

[kariy]

-According to some conversations we had,

i definitely missed the conversation then.

the first iteration consisted only of showing the number of felts by contract and the actual size of the file, in future we might include # Cairo steps per contract, Gas consumption, model size, etc.

I think defining the limits statically should suffice instead of having to import from a file. I would not be too concern with future proofing, we can change it as needed as it's pretty straightforward :)

In any case if the limits is not a go. I can delete the 3 messages and only show number of felts by contract and size file.

though i believe the limits would be useful, i just don't think showing it by default is a nice approach. perhaps we can use a different flag for showing the stats against the limits, and keep the --stats purpose to only display the raw stats.

wdyt?

[fabrobles92]

I think defining the limits statically should suffice instead of having to import from a file. I would not be too concern with future proofing, we can change it as needed as it's pretty straightforward :)

Agree

though i believe the limits would be useful, i just don't think showing it by default is a nice approach. perhaps we can use a different flag for showing the stats against the limits, and keep the --stats purpose to only display the raw stats.

I like a lot this approach.

I will do the modifications to this PR to separate the limit/comparison logic from this issue so we only show the raw stats.

Thanks for the responses :D

[codecov]

Codecov Report

Attention: Patch coverage is 98.48485% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 70.51%. Comparing base (89a5b9e) to head (7e0f742).

Files	Patch %	Lines
bin/sozo/src/commands/build.rs	97.59%	2 Missing ⚠️
crates/sozo/ops/src/statistics.rs	99.13%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1799      +/-   ##
==========================================
+ Coverage   70.34%   70.51%   +0.16%     
==========================================
  Files         308      309       +1     
  Lines       34826    35024     +198     
==========================================
+ Hits        24500    24696     +196     
- Misses      10326    10328       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[lambda-0x]

few thoughts:

[kariy]

I will do the modifications to this PR to separate the limit/comparison logic from this issue so we only show the raw stats.

Yup, i think this PR is good. lets keep it simple and focus only on displaying the stats. We can think about how to integrate the limits stuff in a different one.

[kariy]

i think displaying the stats in a table would look nice and appear much more organized.

+---------------------------+---------------+------------+
|         Contract          | Bytecode size | Class size |
+---------------------------+---------------+------------+
| dojo_examples::contract_1 |       1124123 |      12313 |
| dojo_examples::contract_2 |       1231231 |    1212313 |
| dojo_examples::contract_3 |          1231 |     123124 |
+---------------------------+---------------+------------+

wdyt?

cc @glihm @lambda-0x

[glihm]

i think displaying the stats in a table would look nice and appear much more organized.

+---------------------------+---------------+------------+
|         Contract          | Bytecode size | Class size |
+---------------------------+---------------+------------+
| dojo_examples::contract_1 |       1124123 |      12313 |
| dojo_examples::contract_2 |       1231231 |    1212313 |
| dojo_examples::contract_3 |          1231 |     123124 |
+---------------------------+---------------+------------+

wdyt?

cc @glihm @lambda-0x

The length of contracts name may be very different based on the fully qualified path. But we have a plan to use namespaces instead. So basically, when namespaces will be implemented, we will only have the namespace name, and the contract name (contract_1 for instance). Without having to care about the fully qualified path.

So I guess in the meantime, we can take the longer name and fit to it. And for the sizes, same thing, we should define a limit in number of digits that we know it will not be reached.

EDIT: to illustrate my comment on the table header:

+---------------------------+-----------------------+--------------------+
|         Contract          | Bytecode size (felts) | Class size (bytes) |
+---------------------------+-----------------------+--------------------+
| dojo_examples::contract_1 |               1124123 |              12313 |
| dojo_examples::contract_2 |               1231231 |            1212313 |
| dojo_examples::contract_3 |                  1231 |             123124 |
+---------------------------+-----------------------+--------------------+

[kariy]

+---------------------------+-----------------------+--------------------+
|         Contract          | Bytecode size (felts) | Class size (bytes) |
+---------------------------+-----------------------+--------------------+
| dojo_examples::contract_1 |               1124123 |              12313 |
| dojo_examples::contract_2 |               1231231 |            1212313 |
| dojo_examples::contract_3 |                  1231 |             123124 |
+---------------------------+-----------------------+--------------------+

i like this.

then perhaps we can do something like this for the limits later:

+---------------------------+-----------------------+--------------------+------------------+
|         Contract          | Bytecode size (felts) | Class size (bytes) | limit percentage |
+---------------------------+-----------------------+--------------------+------------------+
| dojo_examples::contract_1 |               1124123 |              12313 | 50%              |
| dojo_examples::contract_2 |               1231231 |           1212313  | 90%              |
| dojo_examples::contract_3 |                  1231 |            123124  | 110%             |
+---------------------------+-----------------------+--------------------+------------------+

[kariy]

looks good so far.

i dont wanna be a PITA, but have some feedbacks on error handling and also some style nits to maintain some form of code aesthetics

[fabrobles92]

looks good so far.

i dont wanna be a PITA, but have some feedbacks on error handling and also some style nits to maintain some form of code aesthetics

Hey not a PITA at all, I really like and value these comments and tips in order to make my future PR's cleaner.

Thank you very much.

[glihm]

Great job @fabrobles92! Nice to see some tests also to cover the use-cases. 👍

Some comments below for the review:

[glihm]

Great job @fabrobles92!

Looks good to me for a first iteration. @kariy do you have any additional inputs for the scope of this PR?

[kariy]

lgtm.. nice work, thanks!