chore: replace `expect` calls #1220

TropicalDog17 · 2024-07-30T05:19:54Z

Resolves #746

frol

4kb is already not too bad. I have spotted a few more potential improvements that could further drop the size. I believe that if we manage to avoid all instances of string formatting (.expect uses formatting under the hood to include the source file name and line of code), we could see a major drop in size, but for that someone would need to check the produced Wasm file static strings and identify how we can avoid those.

As a rule of thumb, panic_any should not be used here. We should either use near_sdk::env::panic_str or otherwise it is an indicator that .expect is not going to be part of Wasm file anyway.

near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs

near-sdk-macros/src/core_impl/contract_metadata/mod.rs

near-sdk-macros/src/lib.rs

near-sdk/src/environment/env.rs

frol · 2024-07-30T07:13:59Z

near-sdk-macros/src/core_impl/code_generator/impl_item_method_info.rs

+                            Ok(deserialized) => deserialized,
+                            Err(_) => near_sdk::env::panic_str("Failed to deserialize input from JSON.")
+                        },
+                        None => near_sdk::env::panic_str("Expected input since method has arguments.")


Please, use fully-qualified syntax here (add :: in the beginning) since this code is generated into the user code and we want to avoid an unexpected aliasing:

Suggested change

None => near_sdk::env::panic_str("Expected input since method has arguments.")

None => ::near_sdk::env::panic_str("Expected input since method has arguments.")

P.S. Apply this change in other places where applicable (in near-sdk-marcos)

TropicalDog17 · 2024-07-30T09:18:34Z

Hey @frol I've applied changes based on your review, and also further optimize in (8296a73), the result is more significant on macros/decorators-heavy contract like examples/adder, the code size is reduce from 78.0kB to 69,3kB, which is an improvement of about ~11% .

frol

Please, revert 8296a73, it is not helping us.

Can you provide the size changes for adder and fungible token contracts after all the changes (after you address my above comments)?

near-sdk-macros/src/core_impl/code_generator/attr_sig_info.rs

near-sdk-macros/src/core_impl/code_generator/snapshots/callback_args.snap

This reverts commit 8296a73.

TropicalDog17 · 2024-07-30T12:31:35Z

hey @frol here is the report, let me know if the pr need further fix

Contract	Original Size (B)	bd1dbba (B)	Improvement (%)	8405364 - latest (B)	Improvement (%)
adder	78039	69818	10.53%	69333	11.15%
callback-results	84004	79800	5.00%	79800	5.00%
cross-contract-calls (high)	80203	72518	9.58%	72518	9.58%
cross-contract-calls (low)	74003	69792	5.69%	69792	5.69%
factory-contract (high)	179885	167965	6.63%	167965	6.63%
factory-contract (low)	173529	165336	4.72%	165336	4.72%
fungible-token (defi)	99552	91987	7.60%	91987	7.60%
fungible-token	165376	160969	2.66%	160969	2.66%
lockable-fungible-token	120367	111769	7.14%	111769	7.14%
mission-control	116445	108107	7.16%	108107	7.16%
non-fungible-token (approval)	96835	88415	8.70%	88415	8.70%
non-fungible-token	228686	221936	2.95%	221936	2.95%
non-fungible-token (receiver)	96576	88150	8.72%	88150	8.72%
status-message	87770	83848	4.47%	83848	4.47%
test-contract	26688	22490	15.73%	22490	15.73%
versioned	101857	95806	5.94%	95806	5.94%

near-sdk-macros/src/core_impl/code_generator/snapshots/callback_args_vec.snap

frol

@TropicalDog17 Please, address the review comment and fix the issues reported by CI.

TropicalDog17 · 2024-07-31T04:18:17Z

@frol I've fixed based on your review. About the failing CI it seems it also fails on master branch with similar issues, so I guess not because of this PR. I don't have much experience in github actions/CI, so I guess it should be addressed in a seperate PR.

frol · 2024-07-31T06:46:43Z

Yeah, I created an issue for that: #1223

akorchyn · 2024-07-31T09:22:11Z

@TropicalDog17 Sorry for jumping in, I'd like to invite you to participate in the Race of Sloths. Just mention @race-of-sloths user in your GitHub comment or PR description to join!

See how the flow works here: NEAR-DevHub/neardevhub-bos#910

frol

@TropicalDog17 Thanks for working on it! I think this is already good enough

@race-of-sloths score 3

TropicalDog17 · 2024-08-01T02:18:54Z

@race-of-sloths

race-of-sloths · 2024-08-01T02:19:46Z

@TropicalDog17 Thank you for your contribution! Your pull request is now a part of the Race of Sloths!
New Sloth joined the Race! Welcome!

Current status: executed

Reviewer	Score
@frol	3

The average score is 3

@TropicalDog17 check out your results on the Race of Sloths Leaderboard! and in the profile

What is the Race of Sloths

Race of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow

For contributors:

Tag @race-of-sloths inside your pull requests
Wait for the maintainer to review and score your pull request
Check out your position in the Leaderboard
Keep weekly and monthly streaks to reach higher positions
Boast your contributions with a dynamic picture of your Profile

For maintainers:

Score pull requests that participate in the Race of Sloths
Engage contributors with fair scoring and fast responses so they keep their streaks
Promote the Race to the point where the Race starts promoting you
Grow the community of your contributors

Feel free to check our website for additional details!

Bot commands

For contributors
- Include a PR: @race-of-sloths include to enter the Race with your PR
For maintainers:
- Assign points: @race-of-sloths score [1/2/3/5/8/13] to award points based on your assessment.
- Reject this PR: @race-of-sloths exclude to send this PR back to the drawing board.
- Exclude repo: @race-of-sloths pause to stop bot activity in this repo until @race-of-sloths unpause command is called

race-of-sloths · 2024-08-01T02:19:49Z

🔄 The PR has been merged.

Important

This pull request is a part of the Race of Sloths and has not been scored yet. Scoring will close in 24 hours! 🕰️

frol · 2024-08-01T06:56:27Z

@race-of-sloths score 3

race-of-sloths · 2024-08-01T06:57:46Z

🌟 Score recorded!

@frol, thank you for scoring this pull request in the Race of Sloths!

race-of-sloths · 2024-08-01T21:47:50Z

✅ PR is finalized!

Your contribution is much appreciated with a final score of 3!
You have received 40 (30 base + 10 weekly bonus) Sloth points for this contribution

Another weekly streak completed, well done @TropicalDog17! To keep your weekly streak and get another bonus make pull request next week! Looking forward to see you in race-of-sloths

TropicalDog17 added 7 commits July 29, 2024 21:55

use panic_str from near_sdk

2c6e162

avoid unwrap_or_else

fe9e360

further refactor

3c068d9

avoid unwrap_or_else

aaf70b9

update

3fa1bd7

fix match error when inside quote!

5f18d13

update snapshot

f0c34c7

TropicalDog17 mentioned this pull request Jul 30, 2024

Investigate .expect potentially pulling in format code #746

Closed

frol requested changes Jul 30, 2024

View reviewed changes

TropicalDog17 added 4 commits July 30, 2024 14:25

revert changes

456a16b

use fully_qualified path of panic_str

bd1dbba

optimize formatting for callback_vec and callback*

8296a73

update snap

6366386

TropicalDog17 requested a review from frol July 30, 2024 09:19

frol requested changes Jul 30, 2024

View reviewed changes

TropicalDog17 added 3 commits July 30, 2024 18:45

Revert "optimize formatting for callback_vec and callback*"

2c9a37d

This reverts commit 8296a73.

remove format! call, refactor

22febc3

update snap

8405364

frol reviewed Jul 30, 2024

View reviewed changes

near-sdk-macros/src/core_impl/code_generator/snapshots/callback_args_vec.snap Outdated Show resolved Hide resolved

frol requested changes Jul 30, 2024

View reviewed changes

revert 22febc3

23d7ca0

TropicalDog17 force-pushed the remove-expect-formatting branch from 8f60c9d to 23d7ca0 Compare July 31, 2024 03:41

cargo update

b872a33

TropicalDog17 force-pushed the remove-expect-formatting branch from b15ba77 to b872a33 Compare July 31, 2024 04:11

frol marked this pull request as ready for review July 31, 2024 18:44

frol requested review from agostbiro and uint as code owners July 31, 2024 18:44

frol added 2 commits July 31, 2024 20:44

Merge branch 'master' into remove-expect-formatting

a6c98d9

reverted unnecessary indentation change

77ff640

frol approved these changes Jul 31, 2024

View reviewed changes

frol merged commit 9dc8aa0 into near:master Jul 31, 2024
19 checks passed

frol mentioned this pull request Jul 31, 2024

chore: release #1226

Merged

akorchyn mentioned this pull request Aug 13, 2024

chore: updates near-* dependencies to 0.24.0 #1237

Merged

TropicalDog17 mentioned this pull request Sep 13, 2024

[feat] create a log! macro keep-starknet-strange/raito#187

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: replace `expect` calls #1220

chore: replace `expect` calls #1220

TropicalDog17 commented Jul 30, 2024 •

edited by frol

Loading

frol left a comment

frol Jul 30, 2024

TropicalDog17 commented Jul 30, 2024 •

edited

Loading

frol left a comment

TropicalDog17 commented Jul 30, 2024

frol left a comment

TropicalDog17 commented Jul 31, 2024 •

edited

Loading

frol commented Jul 31, 2024

akorchyn commented Jul 31, 2024

frol left a comment

TropicalDog17 commented Aug 1, 2024

race-of-sloths commented Aug 1, 2024 •

edited

Loading

race-of-sloths commented Aug 1, 2024

frol commented Aug 1, 2024

race-of-sloths commented Aug 1, 2024

race-of-sloths commented Aug 1, 2024

	None => near_sdk::env::panic_str("Expected input since method has arguments.")
	None => ::near_sdk::env::panic_str("Expected input since method has arguments.")

chore: replace expect calls #1220

chore: replace expect calls #1220

Conversation

TropicalDog17 commented Jul 30, 2024 • edited by frol Loading

frol left a comment

Choose a reason for hiding this comment

frol Jul 30, 2024

Choose a reason for hiding this comment

TropicalDog17 commented Jul 30, 2024 • edited Loading

frol left a comment

Choose a reason for hiding this comment

TropicalDog17 commented Jul 30, 2024

frol left a comment

Choose a reason for hiding this comment

TropicalDog17 commented Jul 31, 2024 • edited Loading

frol commented Jul 31, 2024

akorchyn commented Jul 31, 2024

frol left a comment

Choose a reason for hiding this comment

TropicalDog17 commented Aug 1, 2024

race-of-sloths commented Aug 1, 2024 • edited Loading

race-of-sloths commented Aug 1, 2024

🔄 The PR has been merged.

frol commented Aug 1, 2024

race-of-sloths commented Aug 1, 2024

🌟 Score recorded!

race-of-sloths commented Aug 1, 2024

✅ PR is finalized!

chore: replace `expect` calls #1220

chore: replace `expect` calls #1220

TropicalDog17 commented Jul 30, 2024 •

edited by frol

Loading

TropicalDog17 commented Jul 30, 2024 •

edited

Loading

TropicalDog17 commented Jul 31, 2024 •

edited

Loading

race-of-sloths commented Aug 1, 2024 •

edited

Loading