Implement submit_proof endpoint#132
Implement submit_proof endpoint#132Obiajulu-gif wants to merge 6 commits intoAlien-Protocol:mainfrom
Conversation
|
@Obiajulu-gif Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a verifier-backed CoreContract with persistent storage, new Proof/PublicSignals types, a typed ContractError enum, a UsernameRegistered contract event, storage helpers, verifier client integration, and tests covering submit_proof success and failure cases. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Core as CoreContract
participant Storage as Storage
participant Verifier as VerifierContract
User->>Core: submit_proof(proof, public_signals)
Core->>Storage: get_root()
Storage-->>Core: current_root
alt root != public_signals.old_root
Core-->>User: error (RootMismatch)
else
Core->>Storage: has_commitment(commitment)
Storage-->>Core: exists?
alt exists
Core-->>User: error (DuplicateCommitment)
else
Core->>Storage: get_verifier()
Storage-->>Core: verifier_address
Core->>Verifier: verify_proof(proof, public_signals)
Verifier-->>Core: bool
alt false
Core-->>User: error (InvalidProof)
else
Core->>Storage: store_commitment(commitment)
Core->>Storage: set_root(new_root)
Core->>Core: emit UsernameRegistered(commitment)
Core-->>User: success
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway-contract/contracts/core_contract/src/lib.rs`:
- Around line 27-33: The init function must be gated by an authorization check
so a front-runner cannot set verifier/root; before calling
storage::is_initialized or storage::set_* add a check that the invoker is an
authorized admin (for example compare env.invoker() to the expected
deployer/admin address or a stored admin value) and panic_with_error! with an
Unauthorized/ContractError if it does not match; keep the rest of init
(storage::is_initialized, storage::set_verifier, storage::set_root) intact and
perform the authorization check at the very start of the init function
(referencing init, storage::is_initialized, storage::set_verifier,
storage::set_root).
- Around line 40-45: The current ordering checks current_root vs
public_signals.old_root first and panics with ContractError::RootMismatch,
preventing DuplicateCommitment from ever being reached; to make
duplicate-commitment take precedence, move the storage::has_commitment(&env,
&public_signals.commitment) check (and its panic
ContractError::DuplicateCommitment) before the root comparison, i.e. perform the
duplicate check first in the function that references current_root and
public_signals.old_root, or alternatively update tests/docs to state that
RootMismatch has precedence if you prefer keeping the existing order.
- Around line 48-53: The current call uses VerifierContractClient::verify_proof
which panics on cross-contract failures; replace it with
VerifierContractClient::try_verify_proof(&proof, &public_signals) and handle the
nested Result by mapping invocation errors and false/ok values into
ContractError variants (e.g., map invocation Err to
ContractError::VerifierInvocationFailed or ContractError::NotInitialized as
appropriate and map a false/Ok(false) to ContractError::InvalidProof) so callers
receive stable ContractError values instead of raw panics; update the
surrounding code that currently unwraps storage::get_verifier and the is_valid
check to use the new try_* result mapping.
In `@gateway-contract/contracts/core_contract/src/storage.rs`:
- Around line 15-40: The instance entries set by set_root and set_verifier and
the per-key persistent entry set by store_commitment must extend their TTLs to
avoid Soroban archiving; after calling
env.storage().instance().set(&DataKey::Root, ...) and
env.storage().instance().set(&DataKey::Verifier, ...) add an instance extend_ttl
call for the same DataKey (Root and Verifier) with a long/maximum TTL, and after
env.storage().persistent().set(&DataKey::Commitment(commitment.clone()), &true)
call the persistent extend_ttl for that same
DataKey::Commitment(commitment.clone()) so each stored item’s TTL is refreshed
and won’t expire.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b7fba62-ad5a-438f-a085-15608d8ce1c0
📒 Files selected for processing (6)
gateway-contract/contracts/core_contract/src/errors.rsgateway-contract/contracts/core_contract/src/events.rsgateway-contract/contracts/core_contract/src/lib.rsgateway-contract/contracts/core_contract/src/storage.rsgateway-contract/contracts/core_contract/src/test.rsgateway-contract/contracts/core_contract/src/types.rs
| pub fn init(env: Env, verifier: Address, root: BytesN<32>) { | ||
| if storage::is_initialized(&env) { | ||
| panic_with_error!(&env, ContractError::AlreadyInitialized); | ||
| } | ||
|
|
||
| env.storage().persistent().set(&key, &data); | ||
| storage::set_verifier(&env, &verifier); | ||
| storage::set_root(&env, &root); |
There was a problem hiding this comment.
Gate init behind authorization.
The first caller permanently chooses verifier and root. That lets a front-runner initialize the contract with malicious values and lock the contract into bad state forever.
🔐 Possible fix
- pub fn init(env: Env, verifier: Address, root: BytesN<32>) {
+ pub fn init(env: Env, admin: Address, verifier: Address, root: BytesN<32>) {
+ admin.require_auth();
if storage::is_initialized(&env) {
panic_with_error!(&env, ContractError::AlreadyInitialized);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn init(env: Env, verifier: Address, root: BytesN<32>) { | |
| if storage::is_initialized(&env) { | |
| panic_with_error!(&env, ContractError::AlreadyInitialized); | |
| } | |
| env.storage().persistent().set(&key, &data); | |
| storage::set_verifier(&env, &verifier); | |
| storage::set_root(&env, &root); | |
| pub fn init(env: Env, admin: Address, verifier: Address, root: BytesN<32>) { | |
| admin.require_auth(); | |
| if storage::is_initialized(&env) { | |
| panic_with_error!(&env, ContractError::AlreadyInitialized); | |
| } | |
| storage::set_verifier(&env, &verifier); | |
| storage::set_root(&env, &root); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 27 - 33,
The init function must be gated by an authorization check so a front-runner
cannot set verifier/root; before calling storage::is_initialized or
storage::set_* add a check that the invoker is an authorized admin (for example
compare env.invoker() to the expected deployer/admin address or a stored admin
value) and panic_with_error! with an Unauthorized/ContractError if it does not
match; keep the rest of init (storage::is_initialized, storage::set_verifier,
storage::set_root) intact and perform the authorization check at the very start
of the init function (referencing init, storage::is_initialized,
storage::set_verifier, storage::set_root).
| if current_root != public_signals.old_root.clone() { | ||
| panic_with_error!(&env, ContractError::RootMismatch); | ||
| } | ||
|
|
||
| if storage::has_commitment(&env, &public_signals.commitment) { | ||
| panic_with_error!(&env, ContractError::DuplicateCommitment); |
There was a problem hiding this comment.
duplicate_commitment_is_rejected cannot pass with this check order.
Once the first submission updates the root, replaying the same proof fails at RootMismatch before DuplicateCommitment. Either swap these branches or update the test/docs to make stale-root precedence explicit.
🔁 If duplicate should win, swap the checks
- if current_root != public_signals.old_root.clone() {
- panic_with_error!(&env, ContractError::RootMismatch);
- }
-
if storage::has_commitment(&env, &public_signals.commitment) {
panic_with_error!(&env, ContractError::DuplicateCommitment);
}
+
+ if current_root != public_signals.old_root.clone() {
+ panic_with_error!(&env, ContractError::RootMismatch);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 40 - 45,
The current ordering checks current_root vs public_signals.old_root first and
panics with ContractError::RootMismatch, preventing DuplicateCommitment from
ever being reached; to make duplicate-commitment take precedence, move the
storage::has_commitment(&env, &public_signals.commitment) check (and its panic
ContractError::DuplicateCommitment) before the root comparison, i.e. perform the
duplicate check first in the function that references current_root and
public_signals.old_root, or alternatively update tests/docs to state that
RootMismatch has precedence if you prefer keeping the existing order.
| let verifier = storage::get_verifier(&env) | ||
| .unwrap_or_else(|| panic_with_error!(&env, ContractError::NotInitialized)); | ||
| let verifier_client = VerifierContractClient::new(&env, &verifier); | ||
| let is_valid = verifier_client.verify_proof(&proof, &public_signals); | ||
| if !is_valid { | ||
| panic_with_error!(&env, ContractError::InvalidProof); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "lib.rs" gateway-contractRepository: Alien-Protocol/Alien-Gateway
Length of output: 287
🏁 Script executed:
wc -l gateway-contract/contracts/core_contract/src/lib.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 126
🏁 Script executed:
head -80 gateway-contract/contracts/core_contract/src/lib.rs | cat -nRepository: Alien-Protocol/Alien-Gateway
Length of output: 3082
🏁 Script executed:
rg "ContractError" gateway-contract/contracts/core_contract/src/lib.rs -A 2 -B 2Repository: Alien-Protocol/Alien-Gateway
Length of output: 1402
🏁 Script executed:
rg "enum ContractError" gateway-contract -A 20Repository: Alien-Protocol/Alien-Gateway
Length of output: 608
🏁 Script executed:
rg "verify_proof" gateway-contract -B 5 -A 5Repository: Alien-Protocol/Alien-Gateway
Length of output: 2986
🏁 Script executed:
rg "VerifierContractClient" gateway-contract -B 2 -A 2Repository: Alien-Protocol/Alien-Gateway
Length of output: 2096
🌐 Web query:
Soroban SDK contractclient cross-contract call error handling Result
💡 Result:
In Soroban Rust, generated ContractClient methods (and env.invoke_contract) panic on any cross-contract failure. To handle errors as a Result, use the try_ variants: client.try_method(...) or env.try_invoke_contract(...). (developers.stellar.org)
What try_ returns (nested Result)
A try_ call returns a nested result you can pattern-match:
match client.try_add(&x, &y) {
Ok(Ok(v)) => { /* call succeeded, value decoded */ }
Ok(Err(_conv_err)) => { /* call ok, but return value decode/conversion failed */ }
Err(Ok(contract_err)) => { /* callee returned a spec-defined contract error */ }
Err(Err(status)) => { /* host/system/unrecognized error (Status) */ }
}This exact shape is shown in the Stellar cross-contract guide. (developers.stellar.org)
Simple fallback pattern
let v = client.try_add(&x, &y).unwrap_or(Ok(0)).unwrap();This avoids panics and defaults to 0 if the call errors. (developers.stellar.org)
Making “callee errors” matchable (spec-defined)
If you want Err(Ok(contract_err)) to be meaningful, the called contract should define errors with #[contracterror] and return Result<_, Error> (or escalate with panic_with_error!), so callers can decode/handle them. (developers.stellar.org)
Citations:
- 1: https://developers.stellar.org/docs/build/guides/conventions/cross-contract
- 2: https://developers.stellar.org/docs/build/guides/conventions/cross-contract
- 3: https://developers.stellar.org/docs/build/guides/conventions/cross-contract
- 4: https://developers.stellar.org/docs/build/smart-contracts/example-contracts/errors
Use try_verify_proof to map cross-contract invocation errors to ContractError.
The code calls verifier_client.verify_proof(...) without error handling. Soroban's generated ContractClient methods panic on any cross-contract failure (invalid address, contract trap, etc.). Replace with verifier_client.try_verify_proof(...) and map the nested Result to an appropriate ContractError variant so callers receive stable errors instead of raw invocation failures.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 48 - 53,
The current call uses VerifierContractClient::verify_proof which panics on
cross-contract failures; replace it with
VerifierContractClient::try_verify_proof(&proof, &public_signals) and handle the
nested Result by mapping invocation errors and false/ok values into
ContractError variants (e.g., map invocation Err to
ContractError::VerifierInvocationFailed or ContractError::NotInitialized as
appropriate and map a false/Ok(false) to ContractError::InvalidProof) so callers
receive stable ContractError values instead of raw panics; update the
surrounding code that currently unwraps storage::get_verifier and the is_valid
check to use the new try_* result mapping.
| pub fn get_root(env: &Env) -> Option<BytesN<32>> { | ||
| env.storage().instance().get(&DataKey::Root) | ||
| } | ||
|
|
||
| pub fn set_root(env: &Env, root: &BytesN<32>) { | ||
| env.storage().instance().set(&DataKey::Root, root); | ||
| } | ||
|
|
||
| pub fn get_verifier(env: &Env) -> Option<Address> { | ||
| env.storage().instance().get(&DataKey::Verifier) | ||
| } | ||
|
|
||
| pub fn set_verifier(env: &Env, verifier: &Address) { | ||
| env.storage().instance().set(&DataKey::Verifier, verifier); | ||
| } | ||
|
|
||
| pub fn has_commitment(env: &Env, commitment: &BytesN<32>) -> bool { | ||
| env.storage() | ||
| .persistent() | ||
| .has(&DataKey::Commitment(commitment.clone())) | ||
| } | ||
|
|
||
| pub fn store_commitment(env: &Env, commitment: &BytesN<32>) { | ||
| env.storage() | ||
| .persistent() | ||
| .set(&DataKey::Commitment(commitment.clone()), &true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n gateway-contract/contracts/core_contract/src/storage.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 1439
🏁 Script executed:
rg "extend_ttl|TTL|ttl" gateway-contract/contracts/core_contract/src/ -iRepository: Alien-Protocol/Alien-Gateway
Length of output: 54
🏁 Script executed:
find gateway-contract/contracts/core_contract/src -name "*.rs" -type fRepository: Alien-Protocol/Alien-Gateway
Length of output: 514
🏁 Script executed:
cat -n gateway-contract/contracts/core_contract/src/lib.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 3091
🌐 Web query:
Soroban contract storage TTL extension requirements instance persistent storage
💡 Result:
Instance storage TTL extension (contract instance)
- What gets extended: All instance storage is stored in a single contract instance ledger entry (one shared TTL), so extending instance TTL extends all instance keys, the contract instance, and (via the same call) the contract code entry. The code + instance are checked/extended independently, so one may bump while the other doesn’t. [1]
- How it works (SDK):
env.storage().instance().extend_ttl(threshold, extend_to)only changes TTL if current TTL <threshold, and then sets it so TTL becomesextend_to(otherwise it’s a no-op). [2]
Persistent storage TTL extension (per key)
- What gets extended: Each persistent entry has its own TTL and must be extended individually (per storage key). [1]
- How it works (SDK):
env.storage().persistent().extend_ttl(&key, threshold, extend_to)only extends if that entry’s TTL <threshold, then sets TTL toextend_to. [2]
Extending TTL from outside the contract (transaction operation)
- You can extend TTL using
ExtendFootprintTTLOp, which extends the entries specified in the operation’s footprint so their TTL becomes at leastextendToledgers from now. [1]
Sources: [1] Stellar Docs – Smart contract state archival; [2] soroban-sdk storage.rs source/docs on extend_ttl.
Extend TTL for long-lived storage entries to prevent state expiry.
Instance storage (Root and Verifier) and persistent storage (Commitments) require explicit TTL extension on Soroban—without it, these entries will expire and be archived. This breaks contract initialization state (Root and Verifier queries will fail) and disables duplicate protection (commitments can be replayed after archival). Add extend_ttl calls in the storage helpers: instance TTL in set_root and set_verifier, and persistent TTL per-key in store_commitment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/storage.rs` around lines 15 -
40, The instance entries set by set_root and set_verifier and the per-key
persistent entry set by store_commitment must extend their TTLs to avoid Soroban
archiving; after calling env.storage().instance().set(&DataKey::Root, ...) and
env.storage().instance().set(&DataKey::Verifier, ...) add an instance extend_ttl
call for the same DataKey (Root and Verifier) with a long/maximum TTL, and after
env.storage().persistent().set(&DataKey::Commitment(commitment.clone()), &true)
call the persistent extend_ttl for that same
DataKey::Commitment(commitment.clone()) so each stored item’s TTL is refreshed
and won’t expire.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
gateway-contract/contracts/core_contract/src/lib.rs (3)
53-59:⚠️ Potential issue | 🟠 MajorUse
try_verify_proofto handle cross-contract invocation errors gracefully.
verifier_client.verify_proof(...)panics on any cross-contract failure (invalid address, contract trap, etc.). Replace withtry_verify_proofand map the nestedResultto an appropriateContractErrorvariant so callers receive stable errors instead of raw invocation failures.🛡️ Suggested pattern
let verifier_client = VerifierContractClient::new(&env, &verifier); - let is_valid = verifier_client.verify_proof(&proof, &public_signals); - if !is_valid { - panic_with_error!(&env, ContractError::InvalidProof); - } + match verifier_client.try_verify_proof(&proof, &public_signals) { + Ok(Ok(true)) => { /* valid proof, continue */ } + Ok(Ok(false)) => panic_with_error!(&env, ContractError::InvalidProof), + _ => panic_with_error!(&env, ContractError::InvalidProof), // or a dedicated VerifierCallFailed variant + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 53 - 59, Replace the direct call to verifier_client.verify_proof(...) which panics on cross-contract failures with the safe non-panicking call verifier_client.try_verify_proof(...); unwrap the outer Result and map any transport/invocation error into an appropriate ContractError variant (e.g., a new or existing variant like ContractError::VerificationInvocationFailed) and map the inner Result<bool, _> to ContractError::InvalidProof when false, preserving the existing get_verifier unwrap (panic_with_error! on NotInitialized) and keeping the same panic_with_error! usage for mapped ContractError values so callers get stable errors rather than raw invocation traps.
45-51:⚠️ Potential issue | 🔴 CriticalDecide intended precedence between
RootMismatchandDuplicateCommitment.The current order checks
RootMismatchbeforeDuplicateCommitment. When a proof is replayed, the root will have changed, soRootMismatchfires first — theDuplicateCommitmentbranch is unreachable in that scenario. If duplicate detection should win, swap the checks; otherwise, update tests/documentation to reflect that stale-root takes precedence.🔁 If duplicate should take precedence, swap the checks
+ if storage::has_commitment(&env, &public_signals.commitment) { + panic_with_error!(&env, ContractError::DuplicateCommitment); + } + if current_root != public_signals.old_root.clone() { panic_with_error!(&env, ContractError::RootMismatch); } - - if storage::has_commitment(&env, &public_signals.commitment) { - panic_with_error!(&env, ContractError::DuplicateCommitment); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 45 - 51, Decide which error should take precedence and implement it: if DuplicateCommitment should win, swap the two checks so you call storage::has_commitment(&env, &public_signals.commitment) first and panic_with_error!(... ContractError::DuplicateCommitment) before comparing current_root to public_signals.old_root; otherwise, leave the order as-is and update tests/docs to assert that panic_with_error!(... ContractError::RootMismatch) occurs on replayed proofs. Locate the conditional block using current_root, public_signals.old_root, storage::has_commitment, and the panic_with_error! calls to make the change and adjust tests accordingly.
32-39:⚠️ Potential issue | 🔴 CriticalGate
initbehind authorization to prevent front-running.The
initfunction allows any caller to permanently setverifierandroot. A front-runner could initialize the contract with malicious values, locking it into a bad state forever. Require authorization from a designated admin before allowing initialization.🔐 Suggested fix
- pub fn init(env: Env, verifier: Address, root: BytesN<32>) { + pub fn init(env: Env, admin: Address, verifier: Address, root: BytesN<32>) { + admin.require_auth(); if storage::is_initialized(&env) { panic_with_error!(&env, ContractError::AlreadyInitialized); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 32 - 39, The init function currently lets anyone set verifier and root; protect it by requiring the transaction caller to be an authorized admin before proceeding: in init (which currently calls storage::is_initialized, storage::set_verifier and storage::set_root) first fetch the configured admin (e.g. via a new storage::get_admin(&env) helper or existing admin accessor), then assert the caller matches the admin using env.invoker() or env.require_auth(admin_address) and return/raise a new ContractError::Unauthorized if not authorized; keep the existing ContractError::AlreadyInitialized check, and add/update storage to store an admin (e.g. storage::set_admin) during deployment or via a controlled setter so init cannot be front-run.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway-contract/contracts/core_contract/src/lib.rs`:
- Around line 3-11: Remove the duplicate module declaration and the unused
imports: delete the standalone "mod events;" declaration and leave only "pub mod
events;", and remove "contracterror" and "contracttype" from the used symbols in
the use soroban_sdk { ... } list so the import line only includes the actually
used items (contract, contractimpl, panic_with_error, Address, BytesN, Env).
- Around line 13-16: Remove the duplicated import blocks and consolidate into a
single use statement that includes the missing symbol contractclient;
specifically, eliminate the redundant import lines that re-import contract,
contractimpl, panic_with_error, Address, BytesN, and Env, and replace them with
one consolidated import containing contract, contractclient, contractimpl,
panic_with_error, Address, BytesN, and Env so only one use declaration remains
(locate the import(s) around the top of the file where symbols like contract,
contractimpl, panic_with_error, Address, BytesN, and Env are currently brought
into scope).
---
Duplicate comments:
In `@gateway-contract/contracts/core_contract/src/lib.rs`:
- Around line 53-59: Replace the direct call to
verifier_client.verify_proof(...) which panics on cross-contract failures with
the safe non-panicking call verifier_client.try_verify_proof(...); unwrap the
outer Result and map any transport/invocation error into an appropriate
ContractError variant (e.g., a new or existing variant like
ContractError::VerificationInvocationFailed) and map the inner Result<bool, _>
to ContractError::InvalidProof when false, preserving the existing get_verifier
unwrap (panic_with_error! on NotInitialized) and keeping the same
panic_with_error! usage for mapped ContractError values so callers get stable
errors rather than raw invocation traps.
- Around line 45-51: Decide which error should take precedence and implement it:
if DuplicateCommitment should win, swap the two checks so you call
storage::has_commitment(&env, &public_signals.commitment) first and
panic_with_error!(... ContractError::DuplicateCommitment) before comparing
current_root to public_signals.old_root; otherwise, leave the order as-is and
update tests/docs to assert that panic_with_error!(...
ContractError::RootMismatch) occurs on replayed proofs. Locate the conditional
block using current_root, public_signals.old_root, storage::has_commitment, and
the panic_with_error! calls to make the change and adjust tests accordingly.
- Around line 32-39: The init function currently lets anyone set verifier and
root; protect it by requiring the transaction caller to be an authorized admin
before proceeding: in init (which currently calls storage::is_initialized,
storage::set_verifier and storage::set_root) first fetch the configured admin
(e.g. via a new storage::get_admin(&env) helper or existing admin accessor),
then assert the caller matches the admin using env.invoker() or
env.require_auth(admin_address) and return/raise a new
ContractError::Unauthorized if not authorized; keep the existing
ContractError::AlreadyInitialized check, and add/update storage to store an
admin (e.g. storage::set_admin) during deployment or via a controlled setter so
init cannot be front-run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 506320e5-9812-4c39-a984-a7642bdba45b
📒 Files selected for processing (2)
gateway-contract/contracts/core_contract/src/events.rsgateway-contract/contracts/core_contract/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- gateway-contract/contracts/core_contract/src/events.rs
| mod errors; | ||
| mod events; | ||
| mod storage; | ||
| mod types; | ||
| pub mod events; | ||
|
|
||
| use soroban_sdk::{ | ||
| contract, contracterror, contractimpl, contracttype, panic_with_error, Address, BytesN, Env, | ||
| }; |
There was a problem hiding this comment.
Fix duplicate module declaration and unused imports causing compile errors.
The pipeline fails with multiple errors:
mod events;(line 4) andpub mod events;(line 7) both declare the same module — remove the duplicate.contracterrorandcontracttypein line 10 are unused — remove them.
🐛 Proposed fix
#![no_std]
mod errors;
-mod events;
+pub mod events;
mod storage;
mod types;
-pub mod events;
-use soroban_sdk::{
- contract, contracterror, contractimpl, contracttype, panic_with_error, Address, BytesN, Env,
-};🧰 Tools
🪛 GitHub Actions: Smart Contracts CI
[error] 7-7: Rust compile error E0428: the module name events is defined multiple times (mod events; and pub mod events;).
[warning] 10-10: Rust warning: unused imports contracterror and contracttype (#[warn(unused_imports)]).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 3 - 11,
Remove the duplicate module declaration and the unused imports: delete the
standalone "mod events;" declaration and leave only "pub mod events;", and
remove "contracterror" and "contracttype" from the used symbols in the use
soroban_sdk { ... } list so the import line only includes the actually used
items (contract, contractimpl, panic_with_error, Address, BytesN, Env).
| #[cfg(test)] | ||
| mod test; | ||
|
|
||
| #[contracttype] | ||
| #[derive(Clone)] | ||
| pub enum DataKey { | ||
| Resolver(BytesN<32>), | ||
| } | ||
| use soroban_sdk::{contract, contractclient, contractimpl, panic_with_error, Address, BytesN, Env}; |
There was a problem hiding this comment.
Remove duplicate imports causing compile errors.
Line 16 re-imports symbols already imported in lines 9-11. The pipeline reports E0252 for each duplicate. Consolidate into a single import statement and add only the missing contractclient.
🐛 Proposed fix — single consolidated import
#[cfg(test)]
mod test;
-use soroban_sdk::{contract, contractclient, contractimpl, panic_with_error, Address, BytesN, Env};
+use soroban_sdk::{contract, contractclient, contractimpl, panic_with_error, Address, BytesN, Env};Combined with the earlier fix (removing lines 9-11), this becomes the sole import.
🧰 Tools
🪛 GitHub Actions: Smart Contracts CI
[error] 16-16: Rust compile error E0252: the macro name contract is defined/imported multiple times. Compiler suggests removing the unnecessary import.
[error] 16-16: Rust compile error E0252: the macro name contractimpl is defined/imported multiple times. Compiler suggests removing the unnecessary import.
[error] 16-16: Rust compile error E0252: the macro name panic_with_error is defined/imported multiple times. Compiler suggests removing the unnecessary import.
[error] 16-16: Rust compile error E0252: the type name Address is defined/imported multiple times. Compiler suggests removing the unnecessary import.
[error] 16-16: Rust compile error E0252: the type name BytesN is defined/imported multiple times. Compiler suggests removing the unnecessary import.
[error] 16-16: Rust compile error E0252: the type name Env is defined/imported multiple times. Compiler suggests removing the unnecessary import.
[warning] 16-16: Rust warning: unused imports contract, contractimpl, panic_with_error, Address, BytesN, and Env (#[warn(unused_imports)]).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 13 - 16,
Remove the duplicated import blocks and consolidate into a single use statement
that includes the missing symbol contractclient; specifically, eliminate the
redundant import lines that re-import contract, contractimpl, panic_with_error,
Address, BytesN, and Env, and replace them with one consolidated import
containing contract, contractclient, contractimpl, panic_with_error, Address,
BytesN, and Env so only one use declaration remains (locate the import(s) around
the top of the file where symbols like contract, contractimpl, panic_with_error,
Address, BytesN, and Env are currently brought into scope).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gateway-contract/contracts/core_contract/src/lib.rs (1)
63-91:⚠️ Potential issue | 🔴 CriticalDelete the stale resolver methods that block
submit_proofcompilation.The
if !is_validblock at line 63 never closes—set_memo(line 65) andresolve(line 77) are misplaced inside it. Both methods referenceDataKey::Resolver,ResolveData, andResolverError, which no longer exist.storage.rsnow definesDataKeyas onlyRoot,Verifier, andCommitment.Remove these two methods and close the
ifblock with a closing brace:Fix unclosed block and remove stale code
let is_valid = verifier_client.verify_proof(&proof, &public_signals); if !is_valid { panic_with_error!(&env, ContractError::InvalidProof); - pub fn set_memo(env: Env, commitment: BytesN<32>, memo_id: u64) { - let key = DataKey::Resolver(commitment); - let mut data = env - .storage() - .persistent() - .get::<DataKey, ResolveData>(&key) - .unwrap_or_else(|| panic_with_error!(&env, ResolverError::NotFound)); - - data.memo = Some(memo_id); - env.storage().persistent().set(&key, &data); - } - - pub fn resolve(env: Env, commitment: BytesN<32>) -> (Address, Option<u64>) { - let key = DataKey::Resolver(commitment); - - match env.storage().persistent().get::<DataKey, ResolveData>(&key) { - Some(data) => (data.wallet, data.memo), - None => panic_with_error!(&env, ResolverError::NotFound), - } + } storage::store_commitment(&env, &public_signals.commitment); storage::set_root(&env, &public_signals.new_root);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 63 - 91, The block starting at the `if !is_valid` check is left unclosed and contains stale resolver methods `set_memo` and `resolve` that reference removed symbols (`DataKey::Resolver`, `ResolveData`, `ResolverError`) and thus break `submit_proof` compilation; close the `if !is_valid { ... }` with a single `}` immediately after the `panic_with_error!(&env, ContractError::InvalidProof);` line and delete the entire `pub fn set_memo(env: Env, commitment: BytesN<32>, memo_id: u64)` and `pub fn resolve(env: Env, commitment: BytesN<32>) -> (Address, Option<u64>)` method definitions so the file no longer references the removed resolver types.
♻️ Duplicate comments (4)
gateway-contract/contracts/core_contract/src/lib.rs (4)
51-56:⚠️ Potential issue | 🟡 MinorDecide which error should win for stale duplicate replays.
With this ordering, replaying an already-used proof after the root has advanced returns
RootMismatchbeforeDuplicateCommitment. If the contract/tests expect duplicate-commitment semantics, swap the checks; otherwise make stale-root precedence explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 51 - 56, Decide and implement the intended precedence between stale-root and duplicate-commitment errors: either swap the two checks so storage::has_commitment(&env, &public_signals.commitment) is evaluated before comparing current_root with public_signals.old_root (thereby returning ContractError::DuplicateCommitment on replayed proofs even when root changed), or keep the current order but make the precedence explicit by adding a clear comment and/or an explicit test asserting RootMismatch wins; update the code around current_root, public_signals.old_root, storage::has_commitment, and the panic_with_error! calls accordingly to reflect the chosen semantics.
3-18:⚠️ Potential issue | 🔴 CriticalRemove the duplicated module and import blocks.
events/typesare each declared twice,soroban_sdkis imported twice, andmod testappears twice. Once the brace mismatch is fixed, these become separate compile blockers;use types::ResolveDataalso looks like stale resolver-state plumbing.✂️ Minimal cleanup
mod errors; -mod events; mod storage; -mod types; pub mod events; pub mod types; -use soroban_sdk::{ - contract, contracterror, contractimpl, contracttype, panic_with_error, Address, BytesN, Env, -}; -use types::ResolveData; +use soroban_sdk::{contract, contractclient, contractimpl, panic_with_error, Address, BytesN, Env}; #[cfg(test)] mod test; - -use soroban_sdk::{contract, contractclient, contractimpl, panic_with_error, Address, BytesN, Env}; @@ -#[cfg(test)] -mod test;Also applies to: 107-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 3 - 18, There are duplicate module declarations and imports: remove the second occurrences so each module is declared once (keep mod events; mod types; keep pub mod events; pub mod types only once) and ensure mod test is only declared inside the #[cfg(test)] block; also collapse the duplicated soroban_sdk use into a single use statement that imports the needed symbols (e.g., contract, contracterror, contractimpl, contracttype, panic_with_error, Address, BytesN, Env, contractclient if used). Finally remove the stale use types::ResolveData line if ResolveData is no longer referenced or move it to the correct module if still needed.
38-45:⚠️ Potential issue | 🔴 Critical
initis still callable by any account.The first caller permanently chooses the verifier and root. Without an auth check at the start of
init, anyone can initialize the contract with hostile values before the intended admin does.🔐 One possible guard
- pub fn init(env: Env, verifier: Address, root: BytesN<32>) { + pub fn init(env: Env, admin: Address, verifier: Address, root: BytesN<32>) { + admin.require_auth(); if storage::is_initialized(&env) { panic_with_error!(&env, ContractError::AlreadyInitialized); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 38 - 45, The init function currently allows any caller to set verifier and root; add an authorization guard at the start of init that checks the transaction invoker (e.g., env.invoker() or env.subject() depending on your runtime API) against the expected admin address and panic with a new ContractError::Unauthorized if it doesn’t match, and perform that check before calling storage::set_verifier and storage::set_root (leave the existing storage::is_initialized check in place); if you don’t yet persist an admin, either add an ADMIN constant or a stored owner address (set at deployment) and reference that in the comparison so only the authorized account can call init.
61-64:⚠️ Potential issue | 🟠 MajorMap verifier invocation failures to a contract error.
This path only handles
false. If the verifier contract traps or the cross-contract call fails,verify_proofcan escape as a raw invocation failure instead of a stableContractError. Prefer the generatedtry_verify_proofpath and map call failures separately fromOk(false).For soroban-sdk 23.0.0, do generated `#[contractclient]` methods panic on cross-contract failures, and what nested `Result` shape does `try_verify_proof` return?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/lib.rs` around lines 61 - 64, Replace the direct call to VerifierContractClient::verify_proof with the generated try_verify_proof and explicitly handle both invocation failures and a false verification: call VerifierContractClient::try_verify_proof(&proof, &public_signals), match the outer Result so that an Err from the cross-contract invocation maps to a stable contract error (e.g. panic_with_error!(&env, ContractError::VerifierInvocationFailed) or a similar ContractError variant), and then match the inner Ok(bool) so that Ok(false) still leads to panic_with_error!(&env, ContractError::InvalidProof) while Ok(true) proceeds normally; update references to VerifierContractClient::new, try_verify_proof, verify_proof, ContractError::InvalidProof, and panic_with_error! accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway-contract/contracts/core_contract/src/lib.rs`:
- Around line 27-34: Close the VerifierContract trait by adding the missing `}`
after the verify_proof signature so the trait and the subsequent `ResolverError`
enum are separate and syntactically valid; ensure the `ResolverError` enum
remains at module scope (do not remove it) so current usages that reference
`ResolverError::NotFound` continue to compile, or if you prefer centralizing
errors instead, add a `NotFound = 6` variant to the centralized `ContractError`
in errors.rs and update the two call-sites to use `ContractError::NotFound`
accordingly.
---
Outside diff comments:
In `@gateway-contract/contracts/core_contract/src/lib.rs`:
- Around line 63-91: The block starting at the `if !is_valid` check is left
unclosed and contains stale resolver methods `set_memo` and `resolve` that
reference removed symbols (`DataKey::Resolver`, `ResolveData`, `ResolverError`)
and thus break `submit_proof` compilation; close the `if !is_valid { ... }` with
a single `}` immediately after the `panic_with_error!(&env,
ContractError::InvalidProof);` line and delete the entire `pub fn set_memo(env:
Env, commitment: BytesN<32>, memo_id: u64)` and `pub fn resolve(env: Env,
commitment: BytesN<32>) -> (Address, Option<u64>)` method definitions so the
file no longer references the removed resolver types.
---
Duplicate comments:
In `@gateway-contract/contracts/core_contract/src/lib.rs`:
- Around line 51-56: Decide and implement the intended precedence between
stale-root and duplicate-commitment errors: either swap the two checks so
storage::has_commitment(&env, &public_signals.commitment) is evaluated before
comparing current_root with public_signals.old_root (thereby returning
ContractError::DuplicateCommitment on replayed proofs even when root changed),
or keep the current order but make the precedence explicit by adding a clear
comment and/or an explicit test asserting RootMismatch wins; update the code
around current_root, public_signals.old_root, storage::has_commitment, and the
panic_with_error! calls accordingly to reflect the chosen semantics.
- Around line 3-18: There are duplicate module declarations and imports: remove
the second occurrences so each module is declared once (keep mod events; mod
types; keep pub mod events; pub mod types only once) and ensure mod test is only
declared inside the #[cfg(test)] block; also collapse the duplicated soroban_sdk
use into a single use statement that imports the needed symbols (e.g., contract,
contracterror, contractimpl, contracttype, panic_with_error, Address, BytesN,
Env, contractclient if used). Finally remove the stale use types::ResolveData
line if ResolveData is no longer referenced or move it to the correct module if
still needed.
- Around line 38-45: The init function currently allows any caller to set
verifier and root; add an authorization guard at the start of init that checks
the transaction invoker (e.g., env.invoker() or env.subject() depending on your
runtime API) against the expected admin address and panic with a new
ContractError::Unauthorized if it doesn’t match, and perform that check before
calling storage::set_verifier and storage::set_root (leave the existing
storage::is_initialized check in place); if you don’t yet persist an admin,
either add an ADMIN constant or a stored owner address (set at deployment) and
reference that in the comparison so only the authorized account can call init.
- Around line 61-64: Replace the direct call to
VerifierContractClient::verify_proof with the generated try_verify_proof and
explicitly handle both invocation failures and a false verification: call
VerifierContractClient::try_verify_proof(&proof, &public_signals), match the
outer Result so that an Err from the cross-contract invocation maps to a stable
contract error (e.g. panic_with_error!(&env,
ContractError::VerifierInvocationFailed) or a similar ContractError variant),
and then match the inner Ok(bool) so that Ok(false) still leads to
panic_with_error!(&env, ContractError::InvalidProof) while Ok(true) proceeds
normally; update references to VerifierContractClient::new, try_verify_proof,
verify_proof, ContractError::InvalidProof, and panic_with_error! accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec76f33c-4d41-417d-bdbe-de5878555246
📒 Files selected for processing (2)
gateway-contract/contracts/core_contract/src/lib.rsgateway-contract/contracts/core_contract/src/types.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- gateway-contract/contracts/core_contract/src/types.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway-contract/contracts/core_contract/src/test.rs`:
- Around line 148-160: The test duplicate_commitment_is_rejected uses the
original fixture so public_signals.old_root no longer matches the contract root
after client.submit_proof; update the second submission's public_signals to use
the current root before calling client.try_submit_proof so the duplicate-check
runs: after client.submit_proof(&proof, &public_signals) read the current root
via client.get_root() (or set public_signals.old_root = bytes(&env, 42) to match
the fixture's new_root) and use that adjusted public_signals with
client.try_submit_proof(&proof, &public_signals) so assert_submit_error checks
for ContractError::DuplicateCommitment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 24c272ed-a563-49bc-bfcf-c0aa96ad8fed
📒 Files selected for processing (4)
gateway-contract/contracts/core_contract/src/events.rsgateway-contract/contracts/core_contract/src/lib.rsgateway-contract/contracts/core_contract/src/test.rsgateway-contract/contracts/core_contract/src/types.rs
✅ Files skipped from review due to trivial changes (1)
- gateway-contract/contracts/core_contract/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- gateway-contract/contracts/core_contract/src/events.rs
- gateway-contract/contracts/core_contract/src/types.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gateway-contract/contracts/core_contract/src/types.rs (1)
35-43:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate
PublicSignalstype definition causes compilation failure.This second
PublicSignalsdefinition conflicts with the first one at lines 11-17. The duplicate causes all the pipeline failures:
- E0428: type defined multiple times
- E0119: conflicting trait implementations
- E0063: missing field
commitmentwhen code uses the 3-field variantThe first definition (lines 11-17) includes the
commitmentfield required bysubmit_proofand the test fixtures. This duplicate definition must be removed.Proposed fix: Remove duplicate definition
-/// Public signals extracted from a Groth16 non-inclusion proof. -/// `old_root` must match the current on-chain SMT root. -/// `new_root` becomes the new SMT root after a successful registration. -#[contracttype] -#[derive(Clone)] -pub struct PublicSignals { - pub old_root: BytesN<32>, - pub new_root: BytesN<32>, -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gateway-contract/contracts/core_contract/src/types.rs` around lines 35 - 43, There are two conflicting definitions of the struct PublicSignals; remove the duplicate 2-field definition so the original PublicSignals (with fields old_root, new_root, and commitment) remains the single source of truth; update the file to delete the second declaration (the one lacking commitment) referenced in the diff so all codepaths using submit_proof and the test fixtures continue to compile against the 3-field PublicSignals type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gateway-contract/contracts/core_contract/src/test.rs`:
- Around line 3-5: Old tests rely on a duplicate 2-field PublicSignals and
undefined helpers (setup_with_root, commitment, dummy_proof) so they will break
after removing the duplicate; update those tests (the ones around the original
lines ~162+) to use the correct 3-field PublicSignals variant from types.rs, and
either implement the missing helper functions (add setup_with_root to initialize
contract state, commitment to compute the expected commitment, and dummy_proof
to return a valid Proof/Proof-like placeholder) or remove the obsolete tests
entirely; ensure any test assertions reference the correct field names of the
3-field PublicSignals and that CoreContract/CoreContractClient usages match
their real APIs.
---
Outside diff comments:
In `@gateway-contract/contracts/core_contract/src/types.rs`:
- Around line 35-43: There are two conflicting definitions of the struct
PublicSignals; remove the duplicate 2-field definition so the original
PublicSignals (with fields old_root, new_root, and commitment) remains the
single source of truth; update the file to delete the second declaration (the
one lacking commitment) referenced in the diff so all codepaths using
submit_proof and the test fixtures continue to compile against the 3-field
PublicSignals type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0741d65f-b770-4601-bef3-1ade3bab37eb
📒 Files selected for processing (2)
gateway-contract/contracts/core_contract/src/test.rsgateway-contract/contracts/core_contract/src/types.rs
| use crate::errors::ContractError; | ||
| use crate::types::{Proof, PublicSignals}; | ||
| use crate::{CoreContract, CoreContractClient}; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for PublicSignals constructions with only old_root and new_root (no commitment)
rg -n -A 3 'PublicSignals\s*\{' gateway-contract/contracts/core_contract/src/test.rs | grep -v commitmentRepository: Alien-Protocol/Alien-Gateway
Length of output: 1668
🏁 Script executed:
# Check the actual PublicSignals definition(s) in types.rs
find gateway-contract -name "types.rs" -type fRepository: Alien-Protocol/Alien-Gateway
Length of output: 295
🏁 Script executed:
# Look at the PublicSignals definitions and their contexts
rg -n "^pub struct PublicSignals" gateway-contract/contracts/core_contract/src/ -A 5Repository: Alien-Protocol/Alien-Gateway
Length of output: 855
🏁 Script executed:
# Check the structure and section boundaries in test.rs to understand which lines belong to which tests
rg -n "^#\[test\]|^fn test_" gateway-contract/contracts/core_contract/src/test.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 2513
🏁 Script executed:
# Look at the first few lines of new test code (around line 88) to see which PublicSignals variant
sed -n '85,100p' gateway-contract/contracts/core_contract/src/test.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 583
🏁 Script executed:
# Look at test code around the old tests section (around line 162) to see which variant
sed -n '160,180p' gateway-contract/contracts/core_contract/src/test.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 613
🏁 Script executed:
# Check if the 3-field PublicSignals (with commitment) is used anywhere in test.rs
rg -n "commitment.*:" gateway-contract/contracts/core_contract/src/test.rs | head -20Repository: Alien-Protocol/Alien-Gateway
Length of output: 54
🏁 Script executed:
# Check if setup_with_root, commitment, and dummy_proof are defined in test.rs
rg -n "^fn setup_with_root|^fn commitment|^fn dummy_proof" gateway-contract/contracts/core_contract/src/test.rsRepository: Alien-Protocol/Alien-Gateway
Length of output: 54
🏁 Script executed:
# Check if these functions are defined anywhere or if they're truly missing
rg -n "fn setup_with_root|fn commitment|fn dummy_proof" gateway-contract/contracts/core_contract/src/Repository: Alien-Protocol/Alien-Gateway
Length of output: 54
Old tests at lines 162+ reference undefined functions and will break when the duplicate PublicSignals is removed.
The duplicate 2-field PublicSignals (at types.rs:40-43) is used exclusively by old tests at lines 162+. These tests also depend on missing helper functions: setup_with_root, commitment, and dummy_proof, which are not defined anywhere in the codebase. Once the duplicate definition is removed, these tests must be either removed or refactored to use the correct 3-field PublicSignals variant and provide the missing helpers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@gateway-contract/contracts/core_contract/src/test.rs` around lines 3 - 5, Old
tests rely on a duplicate 2-field PublicSignals and undefined helpers
(setup_with_root, commitment, dummy_proof) so they will break after removing the
duplicate; update those tests (the ones around the original lines ~162+) to use
the correct 3-field PublicSignals variant from types.rs, and either implement
the missing helper functions (add setup_with_root to initialize contract state,
commitment to compute the expected commitment, and dummy_proof to return a valid
Proof/Proof-like placeholder) or remove the obsolete tests entirely; ensure any
test assertions reference the correct field names of the 3-field PublicSignals
and that CoreContract/CoreContractClient usages match their real APIs.
ryzen-xp
added
Blocked
Code Conflict
2 participants
This pull request refactors and restructures the
core_contractfor the gateway contract, introducing a new modular architecture, improved error handling, event emission, and comprehensive storage management. It also adds a robust test suite to verify contract behavior and edge cases.Core contract redesign and modularization:
CoreContract, splitting logic into separate modules:errors,events,storage, andtypes, and updated the contract interface to use a verifier contract client for proof verification.errors.rsfor common contract error conditions such as already initialized, not initialized, root mismatch, invalid proof, and duplicate commitment.UsernameRegisteredevent inevents.rsto signal successful username registration.Storage and state management:
storage.rs, including root/verifier management, commitment tracking, and initialization checks, using a dedicatedDataKeyenum for storage keys.Types and data structures:
ProofandPublicSignalsstructs intypes.rsto clearly define the data required for proof verification and contract state transitions.Testing and validation:
test.rscovering successful proof submission, invalid proofs, stale roots, and duplicate commitments, using a mock verifier contract for controlled testing scenarios.Closes #57
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores