more checked arith and other lints #3214
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
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. |
brentstone
reviewed
May 13, 2024
crates/governance/src/event.rs
Outdated
| /// Passed proposal | ||
| Passed { | ||
| /// Does the proposal contain code |
brentstone
reviewed
May 13, 2024
| @@ -80,25 +81,27 @@ where | |||
| fn read_bytes(&self, key: &Key) -> Result<Option<Vec<u8>>> { | |||
| 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; | |||
There was a problem hiding this comment.
fine not to use a u64::try_from as in other places? Same for below
There was a problem hiding this comment.
I think it's ok to assume 64bit arch - we'll have a warning if not (#3215)
There was a problem hiding this comment.
also it won't fail on 32bit
brentstone
reviewed
May 13, 2024
| let mut transparent_tx_pool = shielded_tx.sapling_value_balance(); | ||
| // The Sapling value balance adds to the transparent tx pool |
There was a problem hiding this comment.
put the comment above the code? Or remove?
tzemanovic
added a commit
that referenced
this pull request
May 14, 2024
tzemanovic
force-pushed
the
tomas/more-checked-arith
branch
from
ec36ae1
to
31e54b9
Compare
thx! I fixed all the issues in https://github.com/anoma/namada/compare/ec36ae148190e183c70cc0c56171aa3bb8b1d359..31e54b9fcccb58d5628cc0362597d5848b576796 |
tzemanovic
force-pushed
the
tomas/more-checked-arith
branch
from
5c2c4ce
to
860132b
Compare
tzemanovic
added a commit
that referenced
this pull request
May 14, 2024
* 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
tzemanovic
added a commit
that referenced
this pull request
May 14, 2024
brentstone
added a commit
that referenced
this pull request
May 21, 2024
* 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
brentstone
added a commit
that referenced
this pull request
May 21, 2024
tzemanovic
added a commit
that referenced
this pull request
May 21, 2024
* 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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