ref: Privatize and Rename State Fields #500

0xNeshi · 2025-01-17T09:53:09Z

Resolves #447, resolves #386

PR Checklist

Tests
Documentation
Changelog

netlify · 2025-01-17T09:53:27Z

✅ Deploy Preview for contracts-stylus canceled.

Name	Link
🔨 Latest commit	`16fbe67`
🔍 Latest deploy log	https://app.netlify.com/sites/contracts-stylus/deploys/678a2d642834a600080cc72e

codecov · 2025-01-17T09:55:26Z

Codecov Report

Attention: Patch coverage is 96.38554% with 9 lines in your changes missing coverage. Please review.

Project coverage is 87.6%. Comparing base (e005e5b) to head (16fbe67).

Files with missing lines	Patch %	Lines
contracts/src/finance/vesting_wallet.rs	80.0%	2 Missing ⚠️
contracts/src/token/erc20/extensions/metadata.rs	0.0%	2 Missing ⚠️
contracts/src/token/erc721/extensions/metadata.rs	33.3%	2 Missing ⚠️
contracts/src/utils/metadata.rs	0.0%	2 Missing ⚠️
...ontracts/src/token/erc721/extensions/enumerable.rs	94.4%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
contracts/src/access/control.rs	`98.0% <100.0%> (-0.1%)`	⬇️
contracts/src/access/ownable.rs	`93.0% <100.0%> (ø)`
contracts/src/access/ownable_two_step.rs	`98.4% <100.0%> (ø)`
contracts/src/token/erc1155/extensions/burnable.rs	`100.0% <100.0%> (ø)`
...racts/src/token/erc1155/extensions/metadata_uri.rs	`72.0% <100.0%> (ø)`
contracts/src/token/erc1155/extensions/supply.rs	`80.8% <100.0%> (ø)`
...tracts/src/token/erc1155/extensions/uri_storage.rs	`98.0% <100.0%> (ø)`
contracts/src/token/erc1155/mod.rs	`96.8% <100.0%> (ø)`
contracts/src/token/erc20/extensions/burnable.rs	`100.0% <100.0%> (ø)`
contracts/src/token/erc20/extensions/capped.rs	`76.9% <100.0%> (ø)`
... and 18 more

bidzyyys

Looks much better!

bidzyyys · 2025-01-17T12:41:17Z

contracts/src/token/erc721/extensions/consecutive.rs

    }

    /// Maximum size of consecutive token's batch.
    /// This is designed to limit stress on off-chain indexing services that
    /// have to record one entry per token, and have protections against
    /// "unreasonably large" batches of tokens.
-    fn _max_batch_size(&self) -> U96 {
-        self._max_batch_size.get()
+    pub fn _max_batch_size(&self) -> U96 {


Why pub for private function?

One reason is that after the change the compiler started complaining that this function and the field with the same name were unused.

Another reason is that I looked at the Solidity version, and the only truly private function is _nextConsecutiveId - all of the other functions are internal, e.g. see _maxBatchSize

If we are to keep compatibility with Solidity, fn _first_consecutive_id should be marked as pub too (see _firstConsecutiveId)

0xNeshi added 26 commits January 16, 2025 13:50

ref: control

e6dc3d5

ref: clippy now warns on pub with underscore

d9a23a6

ref: ownable_two_step

e88319e

ref: ownable

ae889ac

ref: add pub(crate) to private fields

8def8ae

ref: vesting wallet

c3b94fd

ref: erc20 capped

f7f7719

ref: erc20 flash_mint

18980a0

ref: erc20 metadata

cefbf1d

ref: erc20 permit

69a6a7b

ref: remove {} from empty struct in eip712::tests

bc5ce13

ref: remove {} from empty struct in safe_erc20

d18264e

ref: make ownable field public in own2step

a1539d5

ref: erc20

f81e25a

ref: erc721 consecutive

3820914

ref: erc721 enumerable

c79174b

ref: erc721 metadata

14cc318

ref: erc721 uri_storage

9af8e7b

ref: erc721

684edd1

ref: erc721 tests

823fc99

ref: erc1155 all

33c1c9e

ref: checkpoints

728f241

ref: metadata

9d8a488

ref: nonces

f7b432d

ref: pausable

7f99d9d

ref: bitmap

cb17464

0xNeshi self-assigned this Jan 17, 2025

ref: revert own2step test names

ed07d5b

0xNeshi added 4 commits January 17, 2025 10:56

ref: make Ownable pub in VestingWallet

356f933

ref: revert underscores in int. fn names in consecutive

7730096

ref: make metadata pub(crate) in Erc721metadata

3100306

chore: update changelog

16fbe67

0xNeshi changed the title ~~Renaming contr params~~ ref: Privatize and Rename State Fields Jan 17, 2025

0xNeshi marked this pull request as ready for review January 17, 2025 10:55

0xNeshi requested review from bidzyyys and qalisander as code owners January 17, 2025 10:55

bidzyyys approved these changes Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ref: Privatize and Rename State Fields #500

ref: Privatize and Rename State Fields #500

0xNeshi commented Jan 17, 2025 •

edited

Loading

netlify bot commented Jan 17, 2025 •

edited

Loading

codecov bot commented Jan 17, 2025 •

edited

Loading

bidzyyys left a comment

bidzyyys Jan 17, 2025

0xNeshi Jan 17, 2025 •

edited

Loading

ref: Privatize and Rename State Fields #500

Are you sure you want to change the base?

ref: Privatize and Rename State Fields #500

Conversation

0xNeshi commented Jan 17, 2025 • edited Loading

PR Checklist

netlify bot commented Jan 17, 2025 • edited Loading

✅ Deploy Preview for contracts-stylus canceled.

codecov bot commented Jan 17, 2025 • edited Loading

Codecov Report

bidzyyys left a comment

Choose a reason for hiding this comment

bidzyyys Jan 17, 2025

Choose a reason for hiding this comment

0xNeshi Jan 17, 2025 • edited Loading

Choose a reason for hiding this comment

0xNeshi commented Jan 17, 2025 •

edited

Loading

netlify bot commented Jan 17, 2025 •

edited

Loading

codecov bot commented Jan 17, 2025 •

edited

Loading

0xNeshi Jan 17, 2025 •

edited

Loading