Remo/repackage

[remollemo]

Note

Medium Risk
Moderate risk due to workspace/package restructuring and build/profile changes that can affect compilation, dependency resolution, and the deterministic class-hash contract relied on by deployment logic.

Overview
Refactors the Scarb workspace from a single contracts package into multiple first-class packages (account_factory, earn_reporter, eth_712_account, strategy_implementation) and introduces a shared testing_utils crate, updating imports and tests accordingly and removing most modules from contracts (now primarily primer).

Hardens Primer determinism by pinning contracts build settings/dependencies and adding CI enforcement: PRIMER_CLASS_HASH is promoted to a public constant, a new scripts/verify_primer_class_hash.sh builds in release and checks the computed Primer class hash against an expected value, and the workflow runs this check on every PR.

^{Written by Cursor Bugbot for commit a0e038b. This will update automatically on new commits. Configure here.}

---

This change is 

[ItayRosenberg]

@ItayRosenberg partially reviewed 12 files and all commit messages, and made 7 comments.
Reviewable status: 12 of 36 files reviewed, 7 unresolved discussions (waiting on @remollemo).

---

account_factory/src/utils.cairo line 19 at r1 (raw file):

    0x123e6bc1c14ae9934e933d3f64916a6116dd6b036a922b2b1f0815e0d1d300
    .try_into()
    .unwrap();

isnt the non test should be
0x00123e6bc1c14ae9934e933d3f64916a6116dd6b036a922b2b1f0815e0d1d300

we also have this

Code quote:

#[cfg(target: "test")]
pub(crate) const PRIMER_CLASS_HASH: ClassHash =
    0x0279a9bb18604f4ae57633373d56656063203f236cc5aeceea8f2cf40f6336d7
    .try_into()
    .unwrap();

#[cfg(not(target: "test"))]
pub(crate) const PRIMER_CLASS_HASH: ClassHash =
    0x123e6bc1c14ae9934e933d3f64916a6116dd6b036a922b2b1f0815e0d1d300
    .try_into()
    .unwrap();

---

account_factory/src/utils.cairo line 29 at r1 (raw file):

/// Computes the Pedersen hash on the elements of the span using a hash state.
pub fn compute_pedersen_on_elements(data: Span<felt252>) -> felt252 {

this function is duplicated
can you add a todo to move ot to utils

Code quote:

pub fn compute_pedersen_on_elements(data: Span<felt252>) -> felt252 {

---

account_factory/Scarb.toml line 29 at r1 (raw file):

unstable-add-statements-functions-debug-info = true
unstable-add-statements-code-locations-debug-info = true
inlining-strategy = "avoid"

I think that this is redudnat since you define it in the main scarb toml (this is true for all the new scarb tomls)

Code quote:

[profile.dev.cairo]
unstable-add-statements-functions-debug-info = true
unstable-add-statements-code-locations-debug-info = true
inlining-strategy = "avoid"

---

account_factory/src/test_utils.cairo line 15 at r1 (raw file):

pub(crate) fn APP_GOVERNOR() -> ContractAddress {
    'APP_GOVERNOR'.try_into().unwrap()

do we need to duplicate this?
can we use a common test utils?

The same goes to
compute_pedersen_on_elements and compute_contract_address

something like this
contracts/
├── src/
│ ├── lib.cairo
│ ├── primer/
│ ├── utils.cairo # shared utilities
│ └── test_utils.cairo # shared test helpers

Code quote:

pub(crate) fn APP_GOVERNOR() -> ContractAddress {
    'APP_GOVERNOR'.try_into().unwrap()

---

strategy_implementation/src/test_utils.cairo line 28 at r1 (raw file):

/// compiling the Primer contract.
pub(crate) const PRIMER_CLASS_HASH: starknet::ClassHash =
    0x0279a9bb18604f4ae57633373d56656063203f236cc5aeceea8f2cf40f6336d7

we have this already in the utils
Why the duplication?

#[cfg(target: "test")]
pub(crate) const PRIMER_CLASS_HASH: ClassHash =
    0x0279a9bb18604f4ae57633373d56656063203f236cc5aeceea8f2cf40f6336d7
    .try_into()
    .unwrap();

---

scripts/verify_primer_class_hash.sh line 3 at r1 (raw file):

#!/bin/bash
set -e

do we need to set the scarb to a specicif version?

---

account_factory/src/test.cairo line 326 at r1 (raw file):

    let class_hash = get_class_hash_at_syscall(primer_addr).unwrap();
    assert!(class_hash == dummy_eth_address_contract_class_hash, "class hash mismatch");
}

why this is next to primer?

Code quote:

// Primer tests

#[test]
#[should_panic(expected: 'INVALID_CALLER')]
fn test_primer_update_class_hash_invalid_caller() {
    /// set_class_hash should only be callable by the upgrade account set at construction.
    /// Here we impersonate a different caller and expect the function to panic with
    /// 'INVALID_CALLER'.
    let primer_class = snforge_std::declare("Primer").unwrap().contract_class();
    let (primer_addr, _) = primer_class.deploy(@array![]).unwrap();

    let primer = IPrimerDispatcher { contract_address: primer_addr };
    // Impersonate a non-upgrade caller for the next call.
    starkware_utils_testing::test_utils::cheat_caller_address_once(
        contract_address: primer_addr, caller_address: 0x1.try_into().unwrap(),
    );
    // Attempt to update class hash with the wrong caller - should panic (see attribute above).
    let dummy_eth_address_contract_class_hash = declare_dummy_eth_address_contract();
    primer.set_class_hash(new_class_hash: dummy_eth_address_contract_class_hash);
}

#[test]
fn test_primer_sanity_set_class_hash() {
    /// Happy path: after deployment, impersonate the upgrade account and update class hash.
    /// Verifies the on-chain class hash equals the provided value.
    let primer_class = snforge_std::declare("Primer").unwrap().contract_class();
    let (primer_addr, _) = primer_class.deploy(@array![]).unwrap();

    let primer = IPrimerDispatcher { contract_address: primer_addr };
    // Impersonate the upgrade account (same address used by the test infra for this call).
    // Update the class hash and assert it took effect.
    let dummy_eth_address_contract_class_hash = declare_dummy_eth_address_contract();
    starkware_utils_testing::test_utils::cheat_caller_address_once(
        contract_address: primer_addr, caller_address: get_contract_address(),
    );
    primer.set_class_hash(new_class_hash: dummy_eth_address_contract_class_hash);
    let class_hash = get_class_hash_at_syscall(primer_addr).unwrap();
    assert!(class_hash == dummy_eth_address_contract_class_hash, "class hash mismatch");
}

[remollemo]

@remollemo made 4 comments.
Reviewable status: 12 of 36 files reviewed, 7 unresolved discussions (waiting on @ItayRosenberg).

---

account_factory/Scarb.toml line 29 at r1 (raw file):

Previously, ItayRosenberg wrote…

I think that this is redudnat since you define it in the main scarb toml (this is true for all the new scarb tomls)

I think it was added for the CI to succeed. We'll see.

---

scripts/verify_primer_class_hash.sh line 3 at r1 (raw file):

Previously, ItayRosenberg wrote…

do we need to set the scarb to a specicif version?

it's derived from the .tool_versions IIUC.

---

account_factory/src/test.cairo line 326 at r1 (raw file):

Previously, ItayRosenberg wrote…

why this is next to primer?

I have no idea what you're asking.

---

account_factory/src/utils.cairo line 19 at r1 (raw file):

Previously, ItayRosenberg wrote…

isnt the non test should be
0x00123e6bc1c14ae9934e933d3f64916a6116dd6b036a922b2b1f0815e0d1d300

we also have this

you're aware of the equality 0x00123e6bc1c14ae9934e933d3f64916a6116dd6b036a922b2b1f0815e0d1d300 == 0x123e6bc1c14ae9934e933d3f64916a6116dd6b036a922b2b1f0815e0d1d300
right?

[remollemo]

@remollemo made 1 comment and resolved 1 discussion.
Reviewable status: 12 of 36 files reviewed, 6 unresolved discussions (waiting on @ItayRosenberg).

---

account_factory/src/utils.cairo line 29 at r1 (raw file):

Previously, ItayRosenberg wrote…

this function is duplicated
can you add a todo to move ot to utils

it's already in utils. i will not create a new package for comon utils for this. keeping it duplicated.

[ItayRosenberg]

@ItayRosenberg made 2 comments and resolved 2 discussions.
Reviewable status: 12 of 36 files reviewed, 4 unresolved discussions (waiting on @remollemo).

---

account_factory/Scarb.toml line 29 at r1 (raw file):

Previously, remollemo wrote…

I think it was added for the CI to succeed. We'll see.

I was managed to build locally without it

---

account_factory/src/test.cairo line 326 at r1 (raw file):

Previously, remollemo wrote…

I have no idea what you're asking.

why these tests are not in primer_test as before?

[remollemo]

@remollemo made 4 comments and resolved 1 discussion.
Reviewable status: 10 of 36 files reviewed, 3 unresolved discussions (waiting on @ItayRosenberg and @remollemo).

---

account_factory/Scarb.toml line 29 at r1 (raw file):

Previously, ItayRosenberg wrote…

I was managed to build locally without it

CI. CI. not build. CI.
CI.

---

account_factory/src/test.cairo line 326 at r1 (raw file):

Previously, ItayRosenberg wrote…

why these tests are not in primer_test as before?

ah, now i understand the comment. no good reason.

---

account_factory/src/test_utils.cairo line 15 at r1 (raw file):

Previously, ItayRosenberg wrote…

do we need to duplicate this?
can we use a common test utils?

The same goes to
compute_pedersen_on_elements and compute_contract_address

something like this
contracts/
├── src/
│ ├── lib.cairo
│ ├── primer/
│ ├── utils.cairo # shared utilities
│ └── test_utils.cairo # shared test helpers

we can have either another package common for this, or cross dep, or duplication.
since it's small and trivial, i prefer the latter.

---

strategy_implementation/src/test_utils.cairo line 28 at r1 (raw file):

Previously, ItayRosenberg wrote…

we have this already in the utils
Why the duplication?

#[cfg(target: "test")]
pub(crate) const PRIMER_CLASS_HASH: ClassHash =
    0x0279a9bb18604f4ae57633373d56656063203f236cc5aeceea8f2cf40f6336d7
    .try_into()
    .unwrap();

Done.

[remollemo]

@remollemo resolved 1 discussion.
Reviewable status: 9 of 37 files reviewed, 2 unresolved discussions (waiting on @ItayRosenberg).

[ItayRosenberg]

@ItayRosenberg partially reviewed 20 files and all commit messages, and made 4 comments.
Reviewable status: 29 of 37 files reviewed, 6 unresolved discussions (waiting on @remollemo).

---

account_factory/Scarb.toml line 29 at r1 (raw file):

Previously, remollemo wrote…

CI. CI. not build. CI.
CI.

and it doesnt pass the ci
this is weird
We should ask Balouka about this

---

scripts/verify_primer_class_hash.sh line 32 at r4 (raw file):

else
    echo "FAILURE: Primer class hash mismatch!"
    exit 1

this scropt is part of the CI?
Will it fail the CI?

---

account_factory/src/test.cairo line 7 at r4 (raw file):

    eth_address_to_account, get_event_by_selector, get_event_by_selector_n,
    setup_account_factory_test_env,
};

why not crates?

Code quote:

use account_factory::account_factory::AccountFactory::{AccountClassHashChanged, AccountDeployed};
use account_factory::account_factory::{IAccountFactoryDispatcher, IAccountFactoryDispatcherTrait};
use account_factory::test_utils::{
    APP_GOVERNOR, declare_dummy_eth_address_contract, declare_second_dummy_eth_address_contract,
    eth_address_to_account, get_event_by_selector, get_event_by_selector_n,
    setup_account_factory_test_env,
};

---

strategy_implementation/src/lib.cairo line 3 at r4 (raw file):

pub mod avnu_interface;
pub mod interface;
pub(crate) mod known_addresses;

I like it
but why did you decide to put it here and not in others in the lib and in other libs
for example in lib of account factory
pub(crate) mod test_utils;

Code quote:

pub(crate)

[ItayRosenberg]

@ItayRosenberg reviewed 4 files and made 3 comments.
Reviewable status: 33 of 37 files reviewed, 9 unresolved discussions (waiting on @remollemo).

---

a discussion (no related file):

• Consider adding an explicit release profile (at workspace or at least in contracts/) if class-hash reproducibility is a hard requirement.

---

Scarb.toml line 12 at r4 (raw file):

openzeppelin = "2.0.0"
openzeppelin_upgrades = "2.0.0"
openzeppelin_testing = "3.0.0"

delete

Code quote:

openzeppelin_testing = "3.0.0"

---

earn_reporter/Scarb.toml line 14 at r4 (raw file):

[dev-dependencies]
assert_macros.workspace = true

delete

Code quote:

assert_macros.workspace = true

[remollemo]

@remollemo made 4 comments and resolved 3 discussions.
Reviewable status: 27 of 43 files reviewed, 5 unresolved discussions (waiting on @ItayRosenberg).

---

Scarb.toml line 12 at r4 (raw file):

Previously, ItayRosenberg wrote…

delete

why was it there in the first palce?
deleted

---

earn_reporter/Scarb.toml line 14 at r4 (raw file):

Previously, ItayRosenberg wrote…

delete

ICCL... why was it there in the first place?

---

account_factory/src/test.cairo line 7 at r4 (raw file):

Previously, ItayRosenberg wrote…

why not crates?

I dislike relative imports.
it's better than super (maybe) but still, i

---

account_factory/src/test_utils.cairo line 15 at r1 (raw file):

Previously, remollemo wrote…

we can have either another package common for this, or cross dep, or duplication.
since it's small and trivial, i prefer the latter.

done

[remollemo]

@remollemo made 1 comment and resolved 2 discussions.
Reviewable status: 27 of 43 files reviewed, 3 unresolved discussions (waiting on @ItayRosenberg).

---

scripts/verify_primer_class_hash.sh line 32 at r4 (raw file):

Previously, ItayRosenberg wrote…

this scropt is part of the CI?
Will it fail the CI?

🤦 what do you think?

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

[ItayRosenberg]

@ItayRosenberg partially reviewed 12 files and all commit messages, made 6 comments, and resolved 1 discussion.
Reviewable status: 39 of 43 files reviewed, 7 unresolved discussions (waiting on @remollemo).

---

account_factory/src/test.cairo line 7 at r4 (raw file):

Previously, remollemo wrote…

I dislike relative imports.
it's better than super (maybe) but still, i

you literally changed you entire code to use crates beside here :)

---

testing_utils/src/account_factory_utils.cairo line 1 at r7 (raw file):

use account_factory::utils::{PRIMER_CLASS_HASH, compute_contract_address};

I prefer that the name will be with test in it
for example account_factory_test_utils

---

account_factory/src/test_utils.cairo line 1 at r7 (raw file):

// Re-export shared testing utilities from the testing_utils package.

im not sure why did you delete this file?

---

strategy_implementation/src/test_utils.cairo line 803 at r7 (raw file):

        .unwrap_syscall();
    earn_reporter_addr
}

you could have put it in your common dir but i wont go to the rav about this

Code quote:

/// Deploy the EarnReporter contract and return its address.
pub(crate) fn deploy_earn_reporter(owner: ContractAddress) -> ContractAddress {
    let earn_reporter_class = snforge_std::declare("EarnReporter")
        .unwrap_syscall()
        .contract_class();
    let (earn_reporter_addr, _) = earn_reporter_class
        .deploy(@array![owner.into()])
        .unwrap_syscall();
    earn_reporter_addr
}

[remollemo]

@remollemo made 4 comments and resolved 2 discussions.
Reviewable status: 39 of 43 files reviewed, 5 unresolved discussions (waiting on @ItayRosenberg).

---

account_factory/src/test.cairo line 7 at r4 (raw file):

Previously, ItayRosenberg wrote…

you literally changed you entire code to use crates beside here :)

the stupid agent forgot i told him not to do this. (this is a major drawback of this modus operandi, i have multiple agents and they keep forgetting what i tell them)

---

account_factory/src/test_utils.cairo line 1 at r7 (raw file):

Previously, ItayRosenberg wrote…

im not sure why did you delete this file?

i didn't delete it. i consolidated it. progress, you know.

[remollemo]

@remollemo made 1 comment.
Reviewable status: 39 of 43 files reviewed, 5 unresolved discussions (waiting on @ItayRosenberg).

---

testing_utils/src/account_factory_utils.cairo line 1 at r7 (raw file):

Previously, ItayRosenberg wrote…

I prefer that the name will be with test in it
for example account_factory_test_utils

it is the filename is testing_utils/src/account_factory_utils.cairo
the entire package is called testing_utils

[remollemo]

@remollemo resolved 1 discussion.
Reviewable status: 29 of 43 files reviewed, 4 unresolved discussions (waiting on @ItayRosenberg).

[remollemo]

@remollemo resolved 2 discussions.
Reviewable status: 29 of 43 files reviewed, 2 unresolved discussions (waiting on @ItayRosenberg).

[ItayRosenberg]

@ItayRosenberg partially reviewed 14 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @remollemo).

---

account_factory/src/test.cairo line 7 at r4 (raw file):

Previously, remollemo wrote…

the stupid agent forgot i told him not to do this. (this is a major drawback of this modus operandi, i have multiple agents and they keep forgetting what i tell them)

u should set it in the settings

[remollemo]

@remollemo partially reviewed 43 files and all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @remollemo).