more checked arith and other lints #3214

tzemanovic · 2024-05-09T13:43:13Z

Describe your changes

closes #2555

Indicate on which release or other PRs this topic is based on

0.35.1

Checklist before merging to `draft`

I have added a changelog
Git history is in acceptable state

codecov · 2024-05-09T14:13:15Z

Codecov Report

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

Project coverage is 60.26%. Comparing base (4ed6229) to head (860132b).

Files	Patch %	Lines
crates/namada/src/vm/host_env.rs	82.01%	25 Missing ⚠️
...s/apps/src/lib/node/ledger/shell/finalize_block.rs	60.52%	15 Missing ⚠️
crates/namada/src/ledger/vp_host_fns.rs	63.33%	11 Missing ⚠️
crates/tx/src/types.rs	68.57%	11 Missing ⚠️
crates/apps/src/lib/node/ledger/shell/mod.rs	70.96%	9 Missing ⚠️
crates/governance/src/cli/validation.rs	0.00%	9 Missing ⚠️
crates/apps/src/lib/config/genesis/chain.rs	33.33%	8 Missing ⚠️
crates/apps/src/lib/node/ledger/mod.rs	0.00%	8 Missing ⚠️
crates/namada/src/ledger/mod.rs	38.46%	8 Missing ⚠️
crates/proof_of_stake/src/epoched.rs	50.00%	8 Missing ⚠️
... and 22 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3214      +/-   ##
==========================================
+ Coverage   60.24%   60.26%   +0.01%     
==========================================
  Files         303      303              
  Lines       93191    93455     +264     
==========================================
+ Hits        56145    56322     +177     
- Misses      37046    37133      +87

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

brentstone · 2024-05-13T19:22:35Z

crates/governance/src/event.rs

+    },
    /// Passed proposal
    Passed {
+        /// Does the proposal contain code


add a ? at the end

brentstone · 2024-05-13T19:25:13Z

crates/namada/src/ledger/native_vp/ibc/context.rs

        match self.store.get(key) {
            Some(StorageModification::Write { ref value }) => {
-                let gas = key.len() + value.len();
+                let gas = checked!(key.len() + value.len())? as u64;


fine not to use a u64::try_from as in other places? Same for below

I think it's ok to assume 64bit arch - we'll have a warning if not (#3215)

also it won't fail on 32bit

brentstone · 2024-05-13T19:26:18Z

crates/namada/src/ledger/native_vp/masp.rs

+        let mut transparent_tx_pool = shielded_tx.sapling_value_balance();
        // The Sapling value balance adds to the transparent tx pool


put the comment above the code? Or remove?

tzemanovic · 2024-05-14T15:52:08Z

Looks good to me in general, just left a couple questions.

thx! I fixed all the issues in https://github.com/anoma/namada/compare/ec36ae148190e183c70cc0c56171aa3bb8b1d359..31e54b9fcccb58d5628cc0362597d5848b576796

* tomas/more-checked-arith: changelog: add #3214 doc/sdk: fix bare urls doc/gas: fix broken link apps: fix lint warnings apps: add lints namada: fix lint warnings namada: add lints vp_env: fix lint warnings vp_env: add lints tx_env: add lints ethereum_bridge: fix lint warnings ethereum_bridge: add lints ibc: fix lint warnings ibc: add lints vm_env: fix lint warnings vm_env: add lints state: fix lint warnings state: add lints token: fix lint warnings token: add lints proof_of_stake: fix lint warnings proof_of_stake: add lints governance: fix lint warnings governance: add lints account: fix lint warnings account: add lints shielded_token: fix lint warnings shielded_token: add lints trans_token: add lints parameters: add lints controller: fix lint warnings controller: add lints storage: fix lint warnings storage: add lints vote_ext: add lints tx: fix lints warnings tx: add lints gas: fix lints warning gas: add lints merkle_tree: fix lints warning merkle_tree: add lints crates: update for checked events gas events: use checked arith events: add lints replay_protection: add lints

* origin/tomas/more-checked-arith: changelog: add #3214 doc/sdk: fix bare urls doc/gas: fix broken link apps: fix lint warnings apps: add lints namada: fix lint warnings namada: add lints vp_env: fix lint warnings vp_env: add lints tx_env: add lints ethereum_bridge: fix lint warnings ethereum_bridge: add lints ibc: fix lint warnings ibc: add lints vm_env: fix lint warnings vm_env: add lints state: fix lint warnings state: add lints token: fix lint warnings token: add lints proof_of_stake: fix lint warnings proof_of_stake: add lints governance: fix lint warnings governance: add lints account: fix lint warnings account: add lints shielded_token: fix lint warnings shielded_token: add lints trans_token: add lints parameters: add lints controller: fix lint warnings controller: add lints storage: fix lint warnings storage: add lints vote_ext: add lints tx: fix lints warnings tx: add lints gas: fix lints warning gas: add lints merkle_tree: fix lints warning merkle_tree: add lints crates: update for checked events gas events: use checked arith events: add lints replay_protection: add lints

* tomas/split-up-apps: changelog: add #3259 test/apps_lib: fix the top-level dir check fix paths for split up namada_apps_lib git: ignore the new generated version.rs path symlink proto from `apps_lib` `mv crates/apps/build.rs crates/apps_lib/` `mv crates/apps_lib/src/lib/mod.rs crates/apps_lib/src/lib.rs` `mv crates/apps/src/lib crates/apps_lib/src` apps_lib: add a new crate for apps lib crate fixup! Merge branch 'grarco/masp-fees' (#3217) fixup! Merge branch 'grarco/tx-batch' (#3103) fixup! Merge branch 'grarco/tx-batch' (#3103) fixup! Merge branch 'bat/fix/issue-1796' (#3226) fixup! Merge branch 'tiago/max-proposal-bytes-validation' (#3220) fixup! Merge branch 'tomas/more-checked-arith' (#3214) empty commit changelog: add #3216 deliberatly empty Changelog #2817 added clone to transaction structs

tzemanovic added 12 commits May 9, 2024 15:40

replay_protection: add lints

7eb5a92

events: add lints

61a5661

events: use checked arith

6c1c993

crates: update for checked events gas

6270249

merkle_tree: add lints

4ad1799

merkle_tree: fix lints warning

20a449a

gas: add lints

c95b5c4

gas: fix lints warning

0e70ede

tx: add lints

1b35096

tx: fix lints warnings

8f108b6

vote_ext: add lints

34c0c13

storage: add lints

8e06346

tzemanovic mentioned this pull request May 10, 2024

Draft - released as 0.36.0 #3213

Closed

tzemanovic added 10 commits May 10, 2024 18:15

storage: fix lint warnings

b2ca880

controller: add lints

0103ec0

controller: fix lint warnings

9bd7fac

parameters: add lints

c0e4ae7

trans_token: add lints

5134a6d

shielded_token: add lints

e0e16b2

shielded_token: fix lint warnings

1487ad7

account: add lints

a4c1edf

account: fix lint warnings

a9783e4

governance: add lints

f779c9d

tzemanovic added a commit that referenced this pull request May 13, 2024

changelog: add #3214

ec36ae1

tzemanovic marked this pull request as ready for review May 13, 2024 16:37

tzemanovic requested a review from brentstone May 13, 2024 16:37

brentstone reviewed May 13, 2024

View reviewed changes

tzemanovic added 13 commits May 14, 2024 17:50

vm_env: add lints

293b5ca

vm_env: fix lint warnings

8a31264

ibc: add lints

c68d7c0

ibc: fix lint warnings

2a488d7

ethereum_bridge: add lints

fb7e337

ethereum_bridge: fix lint warnings

9457d73

tx_env: add lints

1a6b619

vp_env: add lints

ee6f0c7

vp_env: fix lint warnings

2083ae4

namada: add lints

97cc412

namada: fix lint warnings

2563b46

apps: add lints

2d1fb65

apps: fix lint warnings

ffc35de

tzemanovic added a commit that referenced this pull request May 14, 2024

changelog: add #3214

31e54b9

tzemanovic force-pushed the tomas/more-checked-arith branch from ec36ae1 to 31e54b9 Compare May 14, 2024 15:51

tzemanovic added the ready for draft label May 14, 2024

tzemanovic added 3 commits May 14, 2024 20:10

doc/gas: fix broken link

bccbef0

doc/sdk: fix bare urls

c05ca33

changelog: add #3214

860132b

tzemanovic force-pushed the tomas/more-checked-arith branch from 5c2c4ce to 860132b Compare May 14, 2024 18:10

tzemanovic added a commit that referenced this pull request May 14, 2024

fixup! Merge branch 'tomas/more-checked-arith' (#3214)

253676e

tzemanovic added the merged to draft label May 14, 2024

tzemanovic mentioned this pull request May 16, 2024

lint dbg and prints #3257

Merged

2 tasks

brentstone added a commit that referenced this pull request May 21, 2024

fixup! Merge branch 'tomas/more-checked-arith' (#3214)

fcf5790

tzemanovic merged commit b4c95c3 into main May 21, 2024

tzemanovic deleted the tomas/more-checked-arith branch May 21, 2024 13:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

more checked arith and other lints #3214

more checked arith and other lints #3214

Uh oh!

tzemanovic commented May 9, 2024 •

edited

Loading

Uh oh!

codecov bot commented May 9, 2024 •

edited

Loading

Uh oh!

brentstone May 13, 2024

Uh oh!

brentstone May 13, 2024

Uh oh!

tzemanovic May 14, 2024

Uh oh!

tzemanovic May 14, 2024

Uh oh!

brentstone May 13, 2024

Uh oh!

tzemanovic commented May 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		let mut transparent_tx_pool = shielded_tx.sapling_value_balance();
		// The Sapling value balance adds to the transparent tx pool

more checked arith and other lints #3214

more checked arith and other lints #3214

Uh oh!

Conversation

tzemanovic commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Indicate on which release or other PRs this topic is based on

Checklist before merging to draft

Uh oh!

codecov bot commented May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

brentstone May 13, 2024

Choose a reason for hiding this comment

Uh oh!

brentstone May 13, 2024

Choose a reason for hiding this comment

Uh oh!

tzemanovic May 14, 2024

Choose a reason for hiding this comment

Uh oh!

tzemanovic May 14, 2024

Choose a reason for hiding this comment

Uh oh!

brentstone May 13, 2024

Choose a reason for hiding this comment

Uh oh!

tzemanovic commented May 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tzemanovic commented May 9, 2024 •

edited

Loading

Checklist before merging to `draft`

codecov bot commented May 9, 2024 •

edited

Loading