Upgrade scarb edition to 2024_07

[ggonzalez94]

Fixes #1048
This PR upgades our codebase to scarb 2024_07 edition. Biggest changes are:

• The prelude is much smaller now, and doesn't include any method to access storage(bummer, I know!). So we have to manually import.
• Storage structs and it's members are not pub by default anymore. So we have to manually define them as pub.
  • This is ok for components(since we want smart contract developers to be able to access compinent's storage variables in their contracts)
  • This is ok for mocks(we don't care)
  • For presets we might want to make it more restrictive, but we can decide in a later issue/PR.

PR Checklist

• Tests
• Added entry to CHANGELOG.md

[ericnordelo]

Hey @ggonzalez94, the PR is looking good. A couple of comments:

• We can consolidate imports into fewer lines (I signaled a couple of places, but this applies to the entire PR).
• Note that while adding pub to mocks (and presets) Storage struct makes sense because we access the storage directly in tests, generally, we shouldn't make contracts Storage public, since they are usually not accessed outside the contract itself. I think we shouldn't update doc site examples making the Storage struct public for example.

[ericnordelo]

Suggested change

	use starknet::storage::Map;
	use starknet::storage::StorageMapReadAccess;
	use starknet::storage::StorageMapWriteAccess;
	use starknet::storage::{Map, StorageMapReadAccess, StorageMapWriteAccess};

[ericnordelo]

Suggested change

	use starknet::storage::StoragePointerReadAccess;
	use starknet::storage::StoragePointerWriteAccess;
	use starknet::storage::{StoragePointerReadAccess, StoragePointerWriteAccess};

[ggonzalez94]

@ericnordelo thanks for taking a look even when this was in draft!

We can consolidate imports into fewer lines (I signaled a couple of places, but this applies to the entire PR).

Agreed! This was just the IDE autocompletion, I was already planning to refactor this

Note that while adding pub to mocks (and presets) Storage struct makes sense because we access the storage directly in tests, generally, we shouldn't make contracts Storage public, since they are usually not accessed outside the contract itself. I think we shouldn't update doc site examples making the Storage struct public for example

When I applied the changes I did it so that it compiles with the edition changes and then we can figure out the best way forward. It is my understanding that before this edition the compiler automatically adds the pub modifier to struct and fields, is that correct? If that's the case then if we want to avoid breaking changes we should apply the same to our components.
If we want to take a more thoughtful decision we should consider, what fields of components we want contracts that embed them have direct access to and fields we want to prevent developers to access? Right now my understanding is that since these are pubby default they could access any field from the contract.

[ericnordelo]

Even when this does not change the library's public API, we should add an entry to the CHANGELOG for consistency.

[ggonzalez94]

Even when this does not change the library's public API, we should add an entry to the CHANGELOG for consistency.

Absolutely! I was just waiting for #1137 to get merged to avoid merge conflicts. I added a simple entry under the Changed section. Let me know if you think we should add more details.

[ericnordelo]

LGTM!

[andrew-fleming]

The PR looks good! I left a couple questions

[andrew-fleming]

Would it be better if we make empty storage structs not public?

[ggonzalez94]

I don't think it makes much of a difference, right? But it might be easy to overlook adding the pub keyword if we add some field to it. I can remove all pub from all empty storage structs if you feel strongly about this.

[andrew-fleming]

I don't think it makes much of a difference, right?

Yeahhh this really just occurs with mocks, right? No strong opinions on this. I think it's okay as is

[andrew-fleming]

LGTM!