tests/integration/subscription_to_blocks.rs (1)

397-397: Add a tracked removal target for this temporary ignore.

Line 397 removes coverage for an important fork-range validation path; a bare TODO is easy to miss. Please include a linked issue/milestone in the comment so re-enabling is enforceable.

I can draft a short issue template with clear unignore criteria if helpful.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration/subscription_to_blocks.rs` at line 397, The #[ignore]
attribute on the test in tests/integration/subscription_to_blocks.rs is a
temporary suppression and needs a tracked removal target; replace the bare TODO
with a one-line comment directly above the #[ignore] that references a specific
issue/milestone (e.g. "TODO: Remove after mainnet release — tracked at
ISSUE-123: <link>") so the ignore is discoverable and enforceable, and include
an unignore criteria or milestone date in that comment.

crates/starknet-devnet-core/src/starknet/state_update.rs (1)

124-145: Add an assertion for declared_classes in the unrelated-address branch.

This branch verifies address-scoped fields are empty, but it doesn’t check that address-agnostic declared_classes are preserved. Adding that assertion will prevent silent regressions in filtering semantics.

Proposed test assertion

         let other_diff = filtered_by_other.get_state_diff();
+        assert_eq!(
+            other_diff.declared_classes.len(),
+            full_diff.declared_classes.len(),
+            "Declared classes should remain unchanged by address filtering"
+        );
         assert!(other_diff.nonces.is_empty(), "Unrelated address should have no nonces");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-core/src/starknet/state_update.rs` around lines 124 -
145, The unrelated-address test branch misses asserting that address-agnostic
declared_classes are preserved; update the assertion block around
filtered_by_other/get_state_diff() (other_diff) to also assert that
other_diff.declared_classes equals the original declared classes (or is not
empty as appropriate) so declared_classes are not filtered out by
block_state_update/BlockId::Tag(BlockTag::Latest) when passing
unrelated_address; reference other_diff.declared_classes and ensure the test
verifies declared_classes remain present and unchanged.

crates/starknet-devnet-types/src/rpc/state.rs (1)

92-97: Use a set for membership checks in filter_by_address.

Line 94–97 currently performs repeated linear contains scans on Vec<ContractAddress>, which scales poorly as inputs grow. Converting addresses once to a HashSet keeps filtering predictable and faster.

♻️ Proposed refactor

 pub fn filter_by_address(&self, contract_addresses: Vec<ContractAddress>) -> Self {
+    let address_set: std::collections::HashSet<_> = contract_addresses.into_iter().collect();
     let mut result = self.clone();
-    result.deployed_contracts.retain(|c| contract_addresses.contains(&c.address));
-    result.storage_diffs.retain(|s| contract_addresses.contains(&s.address));
-    result.nonces.retain(|n| contract_addresses.contains(&n.contract_address));
-    result.replaced_classes.retain(|r| contract_addresses.contains(&r.contract_address));
+    result.deployed_contracts.retain(|c| address_set.contains(&c.address));
+    result.storage_diffs.retain(|s| address_set.contains(&s.address));
+    result.nonces.retain(|n| address_set.contains(&n.contract_address));
+    result.replaced_classes.retain(|r| address_set.contains(&r.contract_address));
     result
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-types/src/rpc/state.rs` around lines 92 - 97,
filter_by_address currently does repeated Vec::contains scans on
contract_addresses which is O(n*m); convert contract_addresses into a HashSet at
the start of the function and use set membership checks for filtering. In
practice: inside filter_by_address, create a HashSet<ContractAddress> from the
incoming contract_addresses and then call retain on deployed_contracts,
storage_diffs, nonces, and replaced_classes using set.contains(&c.address) /
set.contains(&s.address) / set.contains(&n.contract_address) /
set.contains(&r.contract_address) respectively to make the filters O(1) per
check.

crates/starknet-devnet-server/src/api/error.rs (1)

88-91: Please pin the new proof error mappings with unit tests.

This file already locks down most public RPC errors with focused assertions, but the new proof-specific branches are still unpinned. A small test each for ProofVerificationFailed -> 69, InvalidProofFacts -> 55 + reason data, and ProvingError -> wildcard would make future spec/code drift much easier to catch.

Also applies to: 231-235, 279-308

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-server/src/api/error.rs` around lines 88 - 91, Add
unit tests that pin the new proof-related RPC error mappings: assert that
ProofVerificationFailed maps to error code 69; assert that InvalidProofFacts
maps to code 55 and that its returned payload includes the provided reason data;
and assert that ProvingError serializes to the generic/wildcard error mapping
(i.e., does not collide with the pinned numeric codes). Place these tests
alongside the existing public RPC error tests in the same test module in
error.rs, using the same helper/serializer used by other tests to produce the
RPC error representation so the assertions target the exact serialized code and
payload.

crates/starknet-devnet-types/src/rpc/proof.rs (1)

15-17: Consider returning &[u32] instead of &Vec<u32> for inner().

Returning a slice (&[u32]) is more idiomatic and flexible than returning &Vec<u32>, as it doesn't expose the underlying container type:

♻️ Suggested improvement

-    pub fn inner(&self) -> &Vec<u32> {
+    pub fn inner(&self) -> &[u32] {
         &self.0
     }

This is a minor API polish suggestion; the current implementation works correctly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-types/src/rpc/proof.rs` around lines 15 - 17, The
method inner() currently returns &Vec<u32>, which unnecessarily exposes the
concrete container; change its signature to return a slice (&[u32]) and return
&self.0[..] (or self.0.as_slice()) so callers get a borrowed slice instead of a
&Vec<u32>; update the inner() definition accordingly to return &[u32] while
keeping the same receiver and behavior.

tests/integration/common/utils.rs (2)

177-192: Consider extracting hardcoded gas parameters to constants.

The gas values (5_000_000, 1_000_000, 2_500_000_000) are test-specific but could benefit from named constants for clarity and easier adjustment if protocol parameters change.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration/common/utils.rs` around lines 177 - 192, Extract the
hardcoded gas numbers into named constants and use them where tx_l1_gas,
tx_l1_data_gas, and tx_l2_gas are defined before calling
account.execute_v3(...). Replace the inline literals (5_000_000, 1_000_000,
2_500_000_000) with descriptive constants like DEFAULT_TEST_TX_L1_GAS,
DEFAULT_TEST_TX_L1_DATA_GAS, and DEFAULT_TEST_TX_L2_GAS (declared at the top of
the test module or file), then leave the call that builds prepared_for_prove and
execute_v3(tx_calls.clone()) unchanged except for using those constants.

---

199-201: Consider documenting why exactly 11 blocks are created.

The magic number 11 for block creation lacks explanation. If this is a protocol requirement (e.g., proof validity window), a comment would improve maintainability. If arbitrary, consider extracting to a named constant.

// Example improvement:
const PROOF_VALIDITY_BLOCKS: usize = 11; // TODO: Document the reason for this value
for _ in 0..PROOF_VALIDITY_BLOCKS {
    devnet.create_block().await.unwrap();
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/integration/common/utils.rs` around lines 199 - 201, The loop calling
devnet.create_block().await.unwrap() uses a magic number 11; replace it with a
named constant (e.g., PROOF_VALIDITY_BLOCKS or BLOCKS_TO_FINALIZE) and add a
brief comment explaining why 11 is required (protocol window, finality, etc.),
then update the for _ in 0..11 to for _ in 0..PROOF_VALIDITY_BLOCKS so the
intent is clear and maintainable.

crates/starknet-devnet-types/src/rpc/transactions/invoke_transaction_v3.rs (1)

55-57: Consider the semantic difference between None and empty Vec in clone_with_proof_facts.

The method uses unwrap_or_default() which converts None to Some(vec![]). This changes the semantics: None (no proof facts provided) becomes Some([]) (explicitly empty proof facts). If this distinction matters for downstream processing, consider preserving None:

💡 Suggested alternative if distinction matters

 pub fn clone_with_proof_facts(&self) -> Self {
-    Self { proof_facts: Some(self.proof_facts.clone().unwrap_or_default()), ..self.clone() }
+    Self { proof_facts: self.proof_facts.clone(), ..self.clone() }
 }

If the intent is to always ensure Some(...) when this method is called, the current implementation is correct.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-types/src/rpc/transactions/invoke_transaction_v3.rs`
around lines 55 - 57, The current clone_with_proof_facts method converts a None
proof_facts into Some(empty Vec) via unwrap_or_default(), changing the semantic
meaning; to preserve the original Option semantics, set proof_facts to
self.proof_facts.clone() instead of
Some(self.proof_facts.clone().unwrap_or_default()), otherwise if the intent
really is to always return Some(...) leave as-is but add a comment clarifying
that None should be normalized to Some([]) for downstream logic; refer to the
clone_with_proof_facts function and the proof_facts field when making this
change.

crates/starknet-devnet-core/src/starknet/events.rs (1)

112-113: Avoid linear address scans in the hot path.

This helper now walks the whole address list for every event. With multi-address filters, get_events scales with events × addresses; normalizing the filter once per request into a set-like structure would keep lookups cheap.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-core/src/starknet/events.rs` around lines 112 - 113,
The current address check in the address_condition match walks the entire
addresses list for every event, causing O(events × addresses) behavior; change
the request-level preprocessing so that the incoming addresses (the addresses
parameter used in get_events) are normalized once into a HashSet (or similar
O(1) lookup structure) and then use set.contains(&event.from_address) in the
address_condition branch instead of address_list.contains(...); update the code
paths that build the filter to accept the precomputed set (or an enum wrapper
for None vs HashSet) so the per-event logic in get_events stays a constant-time
lookup.

crates/starknet-devnet-types/src/rpc/transactions.rs (1)

1133-1168: Consider ordering for determinism.

The conversion iterates over StateMaps fields. If StateMaps uses HashMap internally, the iteration order is non-deterministic. This is fine for informational API responses, but if these initial reads are ever used for proof verification or hashing where deterministic ordering matters, consider sorting the entries or using IndexMap.

Example: Sort storage entries for deterministic output

         storage: value
             .storage
             .iter()
             .map(|s| InitialReadsStorageEntry {
                 contract_address: s.0.0.into(),
                 key: (*s.0.1).into(),
                 value: *s.1,
             })
-            .collect(),
+            .collect::<Vec<_>>()
+            .tap_mut(|v| v.sort_by(|a, b| {
+                (&a.contract_address, &a.key).cmp(&(&b.contract_address, &b.key))
+            })),

Apply similar sorting to other fields if determinism is required.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/starknet-devnet-types/src/rpc/transactions.rs` around lines 1133 -
1168, The conversion impl From<StateMaps> for InitialReads iterates
HashMap-backed StateMaps fields (storage, nonces, class_hashes,
declared_contracts) which yields non-deterministic order; to make InitialReads
deterministic, sort each mapped collection before collecting (e.g., sort storage
entries by (contract_address, key), nonces by contract_address, class_hashes by
contract_address, declared_contracts by class_hash) or use an ordered map
(IndexMap) in StateMaps; update the mapping in InitialReads::from to produce
sorted Vecs for storage, nonces, class_hashes, and declared_contracts so the
output order is stable for hashing/proofs.