feat(bridge): add Axelar cross-chain bridge stubs and design doc#232
Merged
Conversation
📝 WalkthroughWalkthroughThis PR introduces three independently reviewable features: a local Soroban sandbox environment (Makefile and shell orchestration), a cross-chain bridge contract scaffold with Axelar architecture documentation, and a KYC/identity contract scaffold with ZK privacy design guidance. The sandbox automates node startup, contract deployment, and test account funding. The bridge defines message types and error codes. The identity contract sketches KYC status tracking and authorization hooks. ChangesLocal Sandbox Environment Setup
Cross-Chain Bridge Contract and Design
Identity/KYC System and Zero-Knowledge Architecture
Sequence DiagramsequenceDiagram
participant User
participant SandboxScript
participant Docker
participant SorobanCLI
participant LocalFaucet
participant Contracts
participant EnvFile
User->>SandboxScript: make sandbox
SandboxScript->>SandboxScript: check_requirements()
SandboxScript->>Docker: start stellar/quickstart:latest
Docker->>SandboxScript: container ready
SandboxScript->>SandboxScript: generate_keypairs() (Alice, BOB, Admin)
SandboxScript->>LocalFaucet: fund_accounts() (POST to :5000)
LocalFaucet->>SandboxScript: accounts funded
SandboxScript->>SorobanCLI: build_contracts() (cargo wasm32)
SorobanCLI->>SandboxScript: WASM artifacts ready
SandboxScript->>SorobanCLI: deploy_contracts() (soroban contract deploy)
SorobanCLI->>Contracts: deploy to local network
Contracts->>SandboxScript: contract IDs returned
SandboxScript->>EnvFile: write_env_file() (.env.sandbox)
SandboxScript->>User: print_summary() + await SIGINT
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 10
🧹 Nitpick comments (8)
Makefile (1)
9-10: ⚡ Quick winConsider adding error suppression for consistency.
The
sandbox-logstarget will fail with a Docker error if the container doesn't exist, whilesandbox-stopsilently handles this case. For consistent UX, consider adding error suppression.♻️ Proposed fix for consistent error handling
sandbox-logs: - `@docker` logs -f skillsphere-sandbox + `@docker` logs -f skillsphere-sandbox 2>/dev/null || echo "Container not running. Start with 'make sandbox' first."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 9 - 10, The sandbox-logs target currently fails if the Docker container doesn't exist; update the sandbox-logs recipe to suppress/ignore errors like sandbox-stop does so UX is consistent — either prefix the docker command with a Makefile error-ignoring dash or add a shell-level fallback (e.g., append "|| true") to the docker logs invocation in the sandbox-logs target so missing-container errors don't cause the make target to fail.scripts/sandbox.sh (2)
174-183: ⚡ Quick winAdd validation for captured contract IDs.
The deployment output parsing with
tail -1is fragile and doesn't validate that a valid contract ID was captured. If deployment fails or output format changes, invalid IDs could be silently stored.♻️ Proposed fix with validation
CONTRACT_ID=$(echo "${DEPLOY_OUTPUT}" | tail -1) - if [[ -n "${CONTRACT_ID}" ]]; then + if [[ "${CONTRACT_ID}" =~ ^C[A-Z0-9]{55}$ ]]; then CONTRACT_IDS+=("${CONTRACT_ID}") log_info "Deployed ${CONTRACT_NAME}: ${CONTRACT_ID}" + else + log_error "Failed to deploy ${CONTRACT_NAME}. Output: ${DEPLOY_OUTPUT}" + exit 1 fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/sandbox.sh` around lines 174 - 183, The script currently captures the deploy output into DEPLOY_OUTPUT and grabs CONTRACT_ID via tail -1 which is brittle; change the deployment step to (1) check the soroban command exit status and log/deal with failures using DEPLOY_OUTPUT, (2) extract CONTRACT_ID from DEPLOY_OUTPUT using a strict pattern (e.g., regex matching the expected contract ID format/length or known success prefix) instead of tail -1, and (3) only append to CONTRACT_IDS and call log_info when the extracted CONTRACT_ID passes validation; otherwise log an error including DEPLOY_OUTPUT so failures aren’t silently stored.
212-215: ⚡ Quick winEnsure contract naming matches indexer expectations.
The indexer expects
SKILLSPHERE_CONTRACT_IDto be set (seeindexer/src/main.rs:32). The script generates contract environment variables by uppercasing the WASM filename. Verify that the main contract WASM file is namedskillsphere.wasmto ensure proper integration.Alternatively, add an explicit alias after contract deployment:
# After line 215, add: # Alias for main contract (if different name) if [[ -n "${SKILLSPHERE_CONTRACT_ID:-}" ]]; then echo "SKILLSPHERE_CONTRACT_ID=${SKILLSPHERE_CONTRACT_ID}" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/sandbox.sh` around lines 212 - 215, The script currently emits env vars from CONTRACT_NAMES/CONTRACT_IDS by uppercasing WASM filenames, which may not produce SKILLSPHERE_CONTRACT_ID expected by the indexer; either ensure the primary WASM filename is skillsphere.wasm so "${CONTRACT_NAMES[$i]^^}" yields SKILLSPHERE, or add an explicit alias after the deployment loop: if SKILLSPHERE_CONTRACT_ID is set, echo "SKILLSPHERE_CONTRACT_ID=${SKILLSPHERE_CONTRACT_ID}" so the indexer symbol SKILLSPHERE_CONTRACT_ID is always exported; update sandbox.sh around the CONTRACT_NAMES/CONTRACT_IDS loop to implement the chosen fix.contracts/src/identity.rs (1)
6-6: ⚡ Quick winRemove unused import suppression once implementation is complete.
The
#![allow(unused_imports)]attribute is suppressing warnings for imports likesymbol_shortandString(lines 9-10) that are not currently used. While acceptable during initial stub development, this should be removed once the TODOs are implemented or the unused imports should be cleaned up now.🧹 Clean up unused imports
-#![allow(unused_imports)] - use soroban_sdk::{ - contract, contractimpl, contracterror, contracttype, symbol_short, Address, Env, String, + contract, contractimpl, contracterror, contracttype, Address, Env, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/src/identity.rs` at line 6, The crate-level attribute #![allow(unused_imports)] is masking unused-import warnings; remove this attribute (or delete any truly unused imports such as symbol_short and String) once the TODO implementations are complete so the compiler can surface unused imports; locate the attribute at the top of the file and either delete it or remove/use the specific imports (e.g., symbol_short, String) and ensure all remaining imports are actually referenced by functions/structs in this module (like any TODO stubs) before committing.docs/ZK_IDENTITY.md (4)
210-227: 💤 Low valueAdd language identifier to circuit pseudocode block.
The fenced code block contains Circom circuit pseudocode but lacks a language identifier.
🎨 Proposed fix
-``` +```circom // Extended circuit: kyc_credential.circom template KYCCredential() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ZK_IDENTITY.md` around lines 210 - 227, The fenced code block for the Circom circuit is missing a language identifier; update the markdown block containing the KYCCredential template (which references signals like government_id_hash, expiry_date, proof_of_age_threshold, min_age_satisfied and the DeriveAndVerifyAge call) to use ```circom as the opening fence so syntax highlighting and tooling correctly recognize the Circom pseudocode.
175-199: 💤 Low valueAdd language identifier to circuit pseudocode block.
The fenced code block contains Circom circuit pseudocode but lacks a language identifier. Adding
circomorjavascriptwould improve syntax highlighting.🎨 Proposed fix
-``` +```circom // Circom circuit: identity_commitment.circom template IdentityCommitment() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ZK_IDENTITY.md` around lines 175 - 199, The fenced code block for the Circom circuit is missing a language identifier which prevents proper syntax highlighting; update the markdown fence that contains the circuit pseudocode to include a language tag such as "circom" (e.g., ```circom) so the IdentityCommitment template, signals like secret, commitment, nullifier, computed_hash, computed_nullifier, and the component main render with proper highlighting.
120-140: 💤 Low valueAdd language identifier to ASCII diagram code block.
The fenced code block contains an ASCII architecture diagram but lacks a language identifier.
📐 Proposed fix
-``` +```text ┌──────────────┐ ┌─────────────────┐ ┌──────────────────┐ │ User │───►│ Off-chain │───►│ ZK-Prover │🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ZK_IDENTITY.md` around lines 120 - 140, The fenced ASCII diagram block is missing a language identifier; update the triple-backtick fence that surrounds the ASCII diagram (the block starting with "┌──────────────┐ ┌─────────────────┐") to include a language tag like "text" (i.e., change ``` to ```text) so the diagram renders/monospace correctly in the docs.
91-114: 💤 Low valueAdd language identifier to ASCII diagram code block.
The fenced code block contains an ASCII architecture diagram but lacks a language identifier, which causes markdown linters to warn and may affect rendering in some viewers.
📐 Proposed fix
-``` +```text ┌──────────────┐ ┌─────────────────┐ ┌──────────────────┐ │ User │───►│ Off-chain │───►│ ZK-Prover │🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ZK_IDENTITY.md` around lines 91 - 114, The fenced ASCII diagram block lacks a language identifier; update the triple-backtick fence that wraps the diagram in ZK_IDENTITY.md to include a language tag (e.g., ```text) so linters/renderers treat it as plain text; locate the ASCII diagram block (the block starting with the box drawing characters and the “User / Off-chain / ZK-Prover” diagram) and change the opening fence to ```text while leaving the diagram content unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contracts/src/bridge.rs`:
- Around line 61-85: The public bridge entrypoints (e.g.,
receive_bridge_message) currently call todo!() which panics; add a temporary
BridgeError::NotImplemented enum variant and change each stub
(receive_bridge_message and the other stubs noted around lines 101-119, 134-163)
to return Err(BridgeError::NotImplemented) (or the appropriate Result::Err)
instead of invoking todo!(); also update BridgeError definition and any
Display/FromImpls so the error compiles and is returned as a typed contract
error rather than causing a runtime trap.
In `@contracts/src/identity.rs`:
- Around line 118-127: The exported macro require_kyc_if_needed! is currently
empty and will silently skip KYC checks; either remove #[macro_export] until
implemented or implement the check: inside the macro use
$env.storage().persistent().get(&DataKey::AccountKyc($account.clone())) to fetch
Option<KycStatus>, match Some(KycStatus::Required) and return
Err(IdentityError::KycRequired) so callers fail compilation/propagate the error;
ensure the macro's control flow and return type align with callers (i.e., it
returns a Result or uses the same error type) and keep references to
DataKey::AccountKyc, KycStatus, IdentityError::KycRequired, and
$env.storage().persistent().get in the implementation.
- Around line 110-112: The initialize function lacks a one-time guard allowing
re-initialization; update initialize(Env, admin: Address, kyc_oracle: Address)
to check a stored "initialized" flag (e.g., a contract data key or boolean in
storage) and revert if already set, and on first call set the flag to true and
store the admin and kyc_oracle addresses in contract storage (ensuring the same
storage keys used by the contract's admin/kyc accessors); this enforces one-time
initialization and prevents resetting admin or oracle after deployment.
In `@docs/BRIDGE_DESIGN.md`:
- Around line 85-103: The design doc's bridge payload and API descriptions are
out of sync with the implementation: update all references to the payload and
recovery API to match contracts/src/bridge.rs by replacing
destination_chain/destination_address with the actual BridgeMessage fields
(source_chain, source_address, token, recipient, nonce), ensure
receive_bridge_message(BridgeMessage) validation steps reference those fields,
and correct the refund API signature to show request_refund(...)->Result<i128,
BridgeError> and remove any mention of emergency_withdraw if the contract does
not expose it (or add its contract implementation if it should exist); apply the
same corrections to the other referenced sections (e.g., lines ~159-162).
- Around line 52-79: The fenced ASCII diagram blocks (the one showing
ETHEREUM/POLYGON → AXELAR → AXELAR RELAY NETWORK → STELLAR/SOROBAN and the later
similar block) should have their Markdown fences updated to include a language
tag to satisfy markdownlint; change the opening ``` to ```text for the blocks
that contain the ASCII diagram (the block containing symbols like "User Wallet",
"Axelar Gateway", "Axelar validators", and "skillsphere-bridge.rs") and for the
other block around lines noted in the review so both code fences become ```text
without altering the ASCII content.
In `@scripts/sandbox.sh`:
- Around line 128-133: The docker invocation uses both detached mode (-d) and a
trailing background operator (&) which is redundant and confusing; edit the
docker run command that sets CONTAINER_NAME and STELLAR_DOCKER_ARGS (the line
invoking "docker run -d ... stellar/quickstart:latest --standalone &") and
remove the trailing ampersand so the container runs detached solely via -d.
- Line 196: The echo currently uses the GNU-only flag "date -Iseconds" which
breaks on macOS/BSD; update the echo line that contains "$(date -Iseconds)" to
generate an ISO8601/UTC timestamp with portable date format specifiers (use date
with explicit %Y-%m-%dT%H:%M:%S and a UTC indicator) so the script works on both
GNU and BSD/macOS systems.
- Line 163: Replace the global Soroban network registration so the sandbox does
not mutate the user's global config: stop using the `--global` flag in the
`soroban network add --global sandbox local "${NETWORK_URL}"` invocation and
instead either (A) scope the add to a sandbox-specific config dir by running the
add under a sandbox config environment (e.g. set
`SOROBAN_CONFIG_DIR="${SANDBOX_CONFIG_DIR}"` and run `soroban network add
sandbox local "${NETWORK_URL}"`) or (B) avoid `soroban network add` entirely and
pass network parameters directly to deployment commands (use
`--network-passphrase` and `--rpc-url` on `soroban` commands like `soroban
contract deploy`) so the script uses `NETWORK_URL` without altering global
state.
- Around line 72-99: The generate_keypairs function currently falls back to
using `soroban keys generate --global` and sets ALICE_SEC/BOB_SEC/ADMIN_SEC to
empty, which breaks downstream steps and leaks sandbox isolation; change
generate_keypairs to (1) remove the `--global` fallback and instead fail fast if
deterministic generation with `--seed` does not produce expected outputs, (2)
parse `soroban keys generate --seed=... --show-address/--show-secret-from-seed`
output robustly (or use `soroban keys parse`/JSON output if available) to
populate ALICE_PUB/ALICE_SEC, BOB_PUB/BOB_SEC, ADMIN_PUB/ADMIN_SEC, and (3) if
parsing fails, log an error and exit non‑zero so callers of generate_keypairs do
not proceed with empty secret vars; target the generate_keypairs function and
the ALICE_*/BOB_*/ADMIN_* variables when applying these changes.
- Around line 142-146: Update the friendbot curl calls to include the required
addr= query parameter and stop swallowing errors: change the three calls that
use FAUCET_URL with ALICE_PUB, BOB_PUB, and ADMIN_PUB to request
"${FAUCET_URL}?addr=${ALICE_PUB}" (and similarly for BOB_PUB and ADMIN_PUB),
remove the trailing "|| true" so failures propagate, and after each curl call
run the verification command (e.g., stellar account balance --network sandbox
--account "${ALICE_PUB}" for ALICE_PUB, and analogous checks for BOB_PUB and
ADMIN_PUB) so the script exits non‑zero if funding did not succeed.
---
Nitpick comments:
In `@contracts/src/identity.rs`:
- Line 6: The crate-level attribute #![allow(unused_imports)] is masking
unused-import warnings; remove this attribute (or delete any truly unused
imports such as symbol_short and String) once the TODO implementations are
complete so the compiler can surface unused imports; locate the attribute at the
top of the file and either delete it or remove/use the specific imports (e.g.,
symbol_short, String) and ensure all remaining imports are actually referenced
by functions/structs in this module (like any TODO stubs) before committing.
In `@docs/ZK_IDENTITY.md`:
- Around line 210-227: The fenced code block for the Circom circuit is missing a
language identifier; update the markdown block containing the KYCCredential
template (which references signals like government_id_hash, expiry_date,
proof_of_age_threshold, min_age_satisfied and the DeriveAndVerifyAge call) to
use ```circom as the opening fence so syntax highlighting and tooling correctly
recognize the Circom pseudocode.
- Around line 175-199: The fenced code block for the Circom circuit is missing a
language identifier which prevents proper syntax highlighting; update the
markdown fence that contains the circuit pseudocode to include a language tag
such as "circom" (e.g., ```circom) so the IdentityCommitment template, signals
like secret, commitment, nullifier, computed_hash, computed_nullifier, and the
component main render with proper highlighting.
- Around line 120-140: The fenced ASCII diagram block is missing a language
identifier; update the triple-backtick fence that surrounds the ASCII diagram
(the block starting with "┌──────────────┐ ┌─────────────────┐") to include a
language tag like "text" (i.e., change ``` to ```text) so the diagram
renders/monospace correctly in the docs.
- Around line 91-114: The fenced ASCII diagram block lacks a language
identifier; update the triple-backtick fence that wraps the diagram in
ZK_IDENTITY.md to include a language tag (e.g., ```text) so linters/renderers
treat it as plain text; locate the ASCII diagram block (the block starting with
the box drawing characters and the “User / Off-chain / ZK-Prover” diagram) and
change the opening fence to ```text while leaving the diagram content unchanged.
In `@Makefile`:
- Around line 9-10: The sandbox-logs target currently fails if the Docker
container doesn't exist; update the sandbox-logs recipe to suppress/ignore
errors like sandbox-stop does so UX is consistent — either prefix the docker
command with a Makefile error-ignoring dash or add a shell-level fallback (e.g.,
append "|| true") to the docker logs invocation in the sandbox-logs target so
missing-container errors don't cause the make target to fail.
In `@scripts/sandbox.sh`:
- Around line 174-183: The script currently captures the deploy output into
DEPLOY_OUTPUT and grabs CONTRACT_ID via tail -1 which is brittle; change the
deployment step to (1) check the soroban command exit status and log/deal with
failures using DEPLOY_OUTPUT, (2) extract CONTRACT_ID from DEPLOY_OUTPUT using a
strict pattern (e.g., regex matching the expected contract ID format/length or
known success prefix) instead of tail -1, and (3) only append to CONTRACT_IDS
and call log_info when the extracted CONTRACT_ID passes validation; otherwise
log an error including DEPLOY_OUTPUT so failures aren’t silently stored.
- Around line 212-215: The script currently emits env vars from
CONTRACT_NAMES/CONTRACT_IDS by uppercasing WASM filenames, which may not produce
SKILLSPHERE_CONTRACT_ID expected by the indexer; either ensure the primary WASM
filename is skillsphere.wasm so "${CONTRACT_NAMES[$i]^^}" yields SKILLSPHERE, or
add an explicit alias after the deployment loop: if SKILLSPHERE_CONTRACT_ID is
set, echo "SKILLSPHERE_CONTRACT_ID=${SKILLSPHERE_CONTRACT_ID}" so the indexer
symbol SKILLSPHERE_CONTRACT_ID is always exported; update sandbox.sh around the
CONTRACT_NAMES/CONTRACT_IDS loop to implement the chosen fix.
🪄 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: 205e01e7-318d-4d4d-9357-4b32112961b3
📒 Files selected for processing (7)
Makefilecontracts/src/bridge.rscontracts/src/identity.rscontracts/src/lib.rsdocs/BRIDGE_DESIGN.mddocs/ZK_IDENTITY.mdscripts/sandbox.sh
Comment on lines
+61
to
+85
| pub fn receive_bridge_message(env: Env, msg: BridgeMessage) -> Result<(), BridgeError> { | ||
| // TODO: Validate sender is authorized Axelar relayer address | ||
| // let relayer = env | ||
| // .storage() | ||
| // .instance() | ||
| // .get(&DataKey::BridgeRelayer) | ||
| // .ok_or(BridgeError::Unauthorized)?; | ||
| // if env.invoker_contract() != relayer { | ||
| // return Err(BridgeError::Unauthorized); | ||
| // } | ||
|
|
||
| // TODO: Check nonce for replay protection | ||
| // let nonce_key = DataKey::BridgeNonce(msg.source_chain.clone(), msg.source_address.clone(), msg.nonce); | ||
| // if env.storage().persistent().has(&nonce_key) { | ||
| // return Err(BridgeError::ReplayDetected); | ||
| // } | ||
|
|
||
| // TODO: Validate source chain is whitelisted | ||
| // let supported_chains: Vec<String> = env.storage().instance().get(&DataKey::SupportedChains).unwrap_or(Vec::new(&env)); | ||
| // if !supported_chains.contains(&msg.source_chain) { | ||
| // return Err(BridgeError::InvalidChain); | ||
| // } | ||
|
|
||
| todo!() | ||
| } |
There was a problem hiding this comment.
Return a typed contract error from the bridge stubs instead of panicking.
All public entry points currently abort via todo!(), so callers get an opaque trap rather than a BridgeError. Since this module is already exported, that makes the bridge API look supported while every invocation hard-fails at runtime. Either hide the module until it is implemented, or add a temporary NotImplemented variant and return that explicitly from each stub.
Proposed stopgap
pub enum BridgeError {
Unauthorized = 1,
ReplayDetected = 2,
InvalidChain = 3,
InsufficientBalance = 4,
+ NotImplemented = 5,
}
...
- todo!()
+ Err(BridgeError::NotImplemented)Also applies to: 101-119, 134-163
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contracts/src/bridge.rs` around lines 61 - 85, The public bridge entrypoints
(e.g., receive_bridge_message) currently call todo!() which panics; add a
temporary BridgeError::NotImplemented enum variant and change each stub
(receive_bridge_message and the other stubs noted around lines 101-119, 134-163)
to return Err(BridgeError::NotImplemented) (or the appropriate Result::Err)
instead of invoking todo!(); also update BridgeError definition and any
Display/FromImpls so the error compiles and is returned as a typed contract
error rather than causing a runtime trap.
Comment on lines
+110
to
+112
| pub fn initialize(env: Env, admin: Address, kyc_oracle: Address) { | ||
| todo!() | ||
| } |
There was a problem hiding this comment.
Add initialization guard to prevent re-initialization.
The initialize function currently has no protection against being called multiple times, which would allow anyone to reset the admin and oracle addresses after deployment. Standard Soroban initialization patterns use a flag or check to ensure the contract is initialized exactly once.
🔒 Proposed fix to add one-time initialization guard
pub fn initialize(env: Env, admin: Address, kyc_oracle: Address) {
- todo!()
+ // TODO: Check if already initialized
+ // if env.storage().instance().has(&DataKey::Admin) {
+ // panic!("Contract already initialized");
+ // }
+
+ // TODO: Store admin and oracle
+ // env.storage().instance().set(&DataKey::Admin, &admin);
+ // env.storage().instance().set(&DataKey::KycOracle, &kyc_oracle);
+
+ todo!()
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contracts/src/identity.rs` around lines 110 - 112, The initialize function
lacks a one-time guard allowing re-initialization; update initialize(Env, admin:
Address, kyc_oracle: Address) to check a stored "initialized" flag (e.g., a
contract data key or boolean in storage) and revert if already set, and on first
call set the flag to true and store the admin and kyc_oracle addresses in
contract storage (ensuring the same storage keys used by the contract's
admin/kyc accessors); this enforces one-time initialization and prevents
resetting admin or oracle after deployment.
Comment on lines
+118
to
+127
| #[macro_export] | ||
| macro_rules! require_kyc_if_needed { | ||
| ($env:expr, $account:expr) => { | ||
| // TODO: Implement KYC check logic | ||
| // let status: Option<KycStatus> = $env.storage().persistent().get(&DataKey::AccountKyc($account.clone())); | ||
| // if let Some(KycStatus::Required) = status { | ||
| // return Err(IdentityError::KycRequired); | ||
| // } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Exported macro is non-functional and will fail silently.
The require_kyc_if_needed! macro is marked with #[macro_export], making it part of the public API, but its body only contains commented-out TODO code. Any external code attempting to use this macro will receive no compilation error but the KYC check will be silently skipped, creating a security gap.
🚨 Options to address the non-functional macro
Option 1: Remove the export until implementation is ready
-#[macro_export]
+// TODO: Export this macro once implementation is complete
+// #[macro_export]
macro_rules! require_kyc_if_needed {Option 2: Make the macro panic to prevent silent failures
#[macro_export]
macro_rules! require_kyc_if_needed {
($env:expr, $account:expr) => {
- // TODO: Implement KYC check logic
- // let status: Option<KycStatus> = $env.storage().persistent().get(&DataKey::AccountKyc($account.clone()));
- // if let Some(KycStatus::Required) = status {
- // return Err(IdentityError::KycRequired);
- // }
+ compile_error!("require_kyc_if_needed! macro is not yet implemented - remove usage or implement the macro body");
};
}📝 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.
Suggested change
| #[macro_export] | |
| macro_rules! require_kyc_if_needed { | |
| ($env:expr, $account:expr) => { | |
| // TODO: Implement KYC check logic | |
| // let status: Option<KycStatus> = $env.storage().persistent().get(&DataKey::AccountKyc($account.clone())); | |
| // if let Some(KycStatus::Required) = status { | |
| // return Err(IdentityError::KycRequired); | |
| // } | |
| }; | |
| } | |
| // TODO: Export this macro once implementation is complete | |
| // #[macro_export] | |
| macro_rules! require_kyc_if_needed { | |
| ($env:expr, $account:expr) => { | |
| // TODO: Implement KYC check logic | |
| // let status: Option<KycStatus> = $env.storage().persistent().get(&DataKey::AccountKyc($account.clone())); | |
| // if let Some(KycStatus::Required) = status { | |
| // return Err(IdentityError::KycRequired); | |
| // } | |
| }; | |
| } |
Suggested change
| #[macro_export] | |
| macro_rules! require_kyc_if_needed { | |
| ($env:expr, $account:expr) => { | |
| // TODO: Implement KYC check logic | |
| // let status: Option<KycStatus> = $env.storage().persistent().get(&DataKey::AccountKyc($account.clone())); | |
| // if let Some(KycStatus::Required) = status { | |
| // return Err(IdentityError::KycRequired); | |
| // } | |
| }; | |
| } | |
| #[macro_export] | |
| macro_rules! require_kyc_if_needed { | |
| ($env:expr, $account:expr) => { | |
| compile_error!("require_kyc_if_needed! macro is not yet implemented - remove usage or implement the macro body"); | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contracts/src/identity.rs` around lines 118 - 127, The exported macro
require_kyc_if_needed! is currently empty and will silently skip KYC checks;
either remove #[macro_export] until implemented or implement the check: inside
the macro use
$env.storage().persistent().get(&DataKey::AccountKyc($account.clone())) to fetch
Option<KycStatus>, match Some(KycStatus::Required) and return
Err(IdentityError::KycRequired) so callers fail compilation/propagate the error;
ensure the macro's control flow and return type align with callers (i.e., it
returns a Result or uses the same error type) and keep references to
DataKey::AccountKyc, KycStatus, IdentityError::KycRequired, and
$env.storage().persistent().get in the implementation.
Comment on lines
+52
to
+79
| ``` | ||
| ┌─────────────────────────────────────────────────────────────────────────────┐ | ||
| │ ETHEREUM / POLYGON │ | ||
| │ ┌─────────────┐ ┌──────────────┐ │ | ||
| │ │ User Wallet │───►│ Axelar │ │ | ||
| │ │ (USDC) │ │ Gateway │ Locks USDC, emits cross-chain msg │ | ||
| │ └─────────────┘ └──────────────┘ │ | ||
| └──────────────────────────────────┬──────────────────────────────────────────┘ | ||
| │ | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────────────────────┐ | ||
| │ AXELAR RELAY NETWORK │ | ||
| │ ┌─────────────────────────────────────────────────────────────────────┐ │ | ||
| │ │ Axelar validators collect, verify, and forward message with payload │ │ | ||
| │ │ Payload: {source_chain, source_address, token, amount, recipient, nonce}│ │ | ||
| │ └─────────────────────────────────────────────────────────────────────┘ │ | ||
| └──────────────────────────────────┬──────────────────────────────────────────┘ | ||
| │ | ||
| ▼ | ||
| ┌─────────────────────────────────────────────────────────────────────────────┐ | ||
| │ STELLAR / SOROBAN │ | ||
| │ ┌──────────────────┐ ┌───────────────────────────────────────────────┐ │ | ||
| │ │ skillsphere- │───►│ Receives Axelar message │ │ | ||
| │ │ bridge.rs │ │ Validates relayer auth + nonce │ │ | ||
| │ │ │◄───│ Credits recipient account/wallet │ │ | ||
| │ └──────────────────┘ └───────────────────────────────────────────────┘ │ | ||
| └─────────────────────────────────────────────────────────────────────────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add fence languages to the plain-text blocks.
markdownlint is already flagging these blocks. Using text here will clear the warnings without changing the content.
Also applies to: 166-175
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/BRIDGE_DESIGN.md` around lines 52 - 79, The fenced ASCII diagram blocks
(the one showing ETHEREUM/POLYGON → AXELAR → AXELAR RELAY NETWORK →
STELLAR/SOROBAN and the later similar block) should have their Markdown fences
updated to include a language tag to satisfy markdownlint; change the opening
``` to ```text for the blocks that contain the ASCII diagram (the block
containing symbols like "User Wallet", "Axelar Gateway", "Axelar validators",
and "skillsphere-bridge.rs") and for the other block around lines noted in the
review so both code fences become ```text without altering the ASCII content.
Comment on lines
+85
to
+103
| 1. **User Initiation** (Ethereum/Polygon): | ||
| - User calls `IAxelarGateway.approveToken(address(USDC))` to approve USDC spending | ||
| - User calls `IAxelarGateway.payGasForContractCall` with destination = Axelar gateway | ||
| - Payload is serialized: `{destination_chain: "Stellar", destination_address: <Stellar recipient>, amount, nonce}` | ||
| - `usdc.transferFrom(user, gateway, amount)` locks USDC on EVM | ||
|
|
||
| 2. **Axelar Relay**: | ||
| - Axelar validators observe the transaction | ||
| - Payload is gossiped across the Axelar network | ||
| - Threshold signature is assembled for message authenticity | ||
|
|
||
| 3. **Soroban Contract Reception**: | ||
| - `receive_bridge_message(BridgeMessage)` is called by Axelar's trusted relayer | ||
| - Contract validates: | ||
| - Sender is authorized Axelar relayer address | ||
| - Nonce has not been used before (replay protection) | ||
| - Destination chain is supported | ||
| - Contract mints equivalent Stellar-based USDC (or uses existing token handler) | ||
| - Event `bridge_message_received` is emitted |
There was a problem hiding this comment.
Align the design doc with the actual bridge contract API.
The documented payload and recovery surface drift from contracts/src/bridge.rs: the doc uses destination_chain / destination_address, but BridgeMessage expects source_chain, source_address, token, recipient, and nonce; it also documents request_refund(...)->Result<(), BridgeError> plus emergency_withdraw(...), while the contract currently exposes request_refund(...)->Result<i128, BridgeError> and no emergency_withdraw. If an integrator follows this doc, they'll build against the wrong wire format and method contract.
Also applies to: 159-162
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/BRIDGE_DESIGN.md` around lines 85 - 103, The design doc's bridge payload
and API descriptions are out of sync with the implementation: update all
references to the payload and recovery API to match contracts/src/bridge.rs by
replacing destination_chain/destination_address with the actual BridgeMessage
fields (source_chain, source_address, token, recipient, nonce), ensure
receive_bridge_message(BridgeMessage) validation steps reference those fields,
and correct the refund API signature to show request_refund(...)->Result<i128,
BridgeError> and remove any mention of emergency_withdraw if the contract does
not expose it (or add its contract implementation if it should exist); apply the
same corrections to the other referenced sections (e.g., lines ~159-162).
Comment on lines
+72
to
+99
| # Generate deterministic keypairs from fixed seeds | ||
| generate_keypairs() { | ||
| log_info "Generating deterministic test keypairs..." | ||
|
|
||
| # ALICE key - fixed seed for reproducibility | ||
| ALICE_PUB=$(soroban keys generate --seed=27 Alice --show-address 2>/dev/null | grep "Public" | awk '{print $2}') | ||
| ALICE_SEC=$(soroban keys generate --seed=27 Alice --show-secret-from-seed 2>/dev/null | grep "Secret" | awk '{print $2}') | ||
|
|
||
| # BOB key - fixed seed for reproducibility | ||
| BOB_PUB=$(soroban keys generate --seed=53 Bob --show-address 2>/dev/null | grep "Public" | awk '{print $2}') | ||
| BOB_SEC=$(soroban keys generate --seed=53 Bob --show-secret-from-seed 2>/dev/null | grep "Secret" | awk '{print $2}') | ||
|
|
||
| # ADMIN key - fixed seed for reproducibility | ||
| ADMIN_PUB=$(soroban keys generate --seed=91 Admin --show-address 2>/dev/null | grep "Public" | awk '{print $2}') | ||
| ADMIN_SEC=$(soroban keys generate --seed=91 Admin --show-secret-from-seed 2>/dev/null | grep "Secret" | awk '{print $2}') | ||
|
|
||
| # Alternative deterministic generation using openssl and soroban keys parse | ||
| if [[ -z "${ALICE_PUB}" ]]; then | ||
| # Use explicit key generation for sandbox | ||
| log_info "Using built-in key generation for test accounts..." | ||
| ALICE_PUB=$(soroban keys generate --global Alice 2>/dev/null && soroban keys address Alice) | ||
| ALICE_SEC="" | ||
| BOB_PUB=$(soroban keys generate --global Bob 2>/dev/null && soroban keys address Bob) | ||
| BOB_SEC="" | ||
| ADMIN_PUB=$(soroban keys generate --global Admin 2>/dev/null && soroban keys address Admin) | ||
| ADMIN_SEC="" | ||
| fi | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify soroban CLI key generation syntax and output format
# Check soroban keys generate help
soroban keys generate --help 2>&1 | head -30
# Test key generation with seed flag
soroban keys generate --seed=27 test-verify-key 2>&1 || echo "seed flag failed"
# Check available flags for showing keys
soroban keys --help 2>&1 | grep -A5 "address\|show"Repository: LightForgeHub/SkillSphere-Dapp
Length of output: 183
Fix deterministic keypair generation fallback and parsing in scripts/sandbox.sh
- The
if [[ -z "${ALICE_PUB}" ]]fallback usessoroban keys generate --global ...but explicitly setsALICE_SEC/BOB_SEC/ADMIN_SECto"", so any downstream step that needs secret keys will run with empty secrets. - The
soroban keys generate --seed=... --show-address/--show-secret-from-seed | grep/awkapproach depends on fragile, text-output formatting; switch to stable parsing or hard-fail when expected fields aren’t extracted. - Avoid
--globalfor sandbox/test isolation (use sandbox-scoped state/config or remove the--globalfallback).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sandbox.sh` around lines 72 - 99, The generate_keypairs function
currently falls back to using `soroban keys generate --global` and sets
ALICE_SEC/BOB_SEC/ADMIN_SEC to empty, which breaks downstream steps and leaks
sandbox isolation; change generate_keypairs to (1) remove the `--global`
fallback and instead fail fast if deterministic generation with `--seed` does
not produce expected outputs, (2) parse `soroban keys generate --seed=...
--show-address/--show-secret-from-seed` output robustly (or use `soroban keys
parse`/JSON output if available) to populate ALICE_PUB/ALICE_SEC,
BOB_PUB/BOB_SEC, ADMIN_PUB/ADMIN_SEC, and (3) if parsing fails, log an error and
exit non‑zero so callers of generate_keypairs do not proceed with empty secret
vars; target the generate_keypairs function and the ALICE_*/BOB_*/ADMIN_*
variables when applying these changes.
Comment on lines
+128
to
+133
| docker run -d \ | ||
| --name "${CONTAINER_NAME}" \ | ||
| -p 8000:8000 \ | ||
| -p 11626:11626 \ | ||
| -e STELLAR_DOCKER_ARGS="--allow-input-expiration --limit-peer=1000" \ | ||
| stellar/quickstart:latest --standalone & |
There was a problem hiding this comment.
Remove redundant background operator.
Line 133 ends with &, but the docker run -d command already runs in detached mode. The trailing & is redundant and potentially confusing.
🐛 Proposed fix
docker run -d \
--name "${CONTAINER_NAME}" \
-p 8000:8000 \
-p 11626:11626 \
-e STELLAR_DOCKER_ARGS="--allow-input-expiration --limit-peer=1000" \
- stellar/quickstart:latest --standalone &
+ stellar/quickstart:latest --standalone📝 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.
Suggested change
| docker run -d \ | |
| --name "${CONTAINER_NAME}" \ | |
| -p 8000:8000 \ | |
| -p 11626:11626 \ | |
| -e STELLAR_DOCKER_ARGS="--allow-input-expiration --limit-peer=1000" \ | |
| stellar/quickstart:latest --standalone & | |
| docker run -d \ | |
| --name "${CONTAINER_NAME}" \ | |
| -p 8000:8000 \ | |
| -p 11626:11626 \ | |
| -e STELLAR_DOCKER_ARGS="--allow-input-expiration --limit-peer=1000" \ | |
| stellar/quickstart:latest --standalone |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sandbox.sh` around lines 128 - 133, The docker invocation uses both
detached mode (-d) and a trailing background operator (&) which is redundant and
confusing; edit the docker run command that sets CONTAINER_NAME and
STELLAR_DOCKER_ARGS (the line invoking "docker run -d ...
stellar/quickstart:latest --standalone &") and remove the trailing ampersand so
the container runs detached solely via -d.
Comment on lines
+142
to
+146
| # Create friendbot funding requests | ||
| curl -s "${FAUCET_URL}?${ALICE_PUB}" > /dev/null 2>&1 || true | ||
| curl -s "${FAUCET_URL}?${BOB_PUB}" > /dev/null 2>&1 || true | ||
| curl -s "${FAUCET_URL}?${ADMIN_PUB}" > /dev/null 2>&1 || true | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct URL format for Stellar quickstart local friendbot endpoint?
💡 Result:
The correct URL format for the Stellar Quickstart local Friendbot endpoint is http://localhost:8000/friendbot [1][2][3]. When running the Stellar Quickstart container, the service typically binds to port 8000 on your local machine [1][2][3]. You can interact with the Friendbot faucet by sending a request to this endpoint, passing the destination public key as a query parameter [1][2][3]. For example, you can use the following format with curl: curl "http://localhost:8000/friendbot?addr=YOUR_PUBLIC_KEY" [1][2][3] Note that while this is the standard endpoint for the local Quickstart environment, requests to this same URL are automatically proxied to the official Stellar Friendbot deployments when the Quickstart container is running in Testnet or Futurenet modes [1][2][3].
Citations:
- 1: https://github.com/stellar/quickstart/tree/4837d5deb27f617050d72b06b27bb0d371402c4f
- 2: https://github.com/stellar/quickstart/blob/main/README.md
- 3: https://github.com/stellar/quickstart
Fix Friendbot funding request URL + stop silently ignoring failures (scripts/sandbox.sh:142-146).
- Stellar Quickstart’s local Friendbot endpoint uses
?addr=<PUBLIC_KEY>(e.g.,http://localhost:8000/friendbot?addr=YOUR_PUBLIC_KEY), but the script callscurl ... "${FAUCET_URL}?${ALICE_PUB}"(noaddr=), so the funding requests are likely malformed. - Remove
|| trueand add a post-funding check (e.g.,stellar account balance --network sandbox --account "${ALICE_PUB}") that fails the script if accounts aren’t funded.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sandbox.sh` around lines 142 - 146, Update the friendbot curl calls
to include the required addr= query parameter and stop swallowing errors: change
the three calls that use FAUCET_URL with ALICE_PUB, BOB_PUB, and ADMIN_PUB to
request "${FAUCET_URL}?addr=${ALICE_PUB}" (and similarly for BOB_PUB and
ADMIN_PUB), remove the trailing "|| true" so failures propagate, and after each
curl call run the verification command (e.g., stellar account balance --network
sandbox --account "${ALICE_PUB}" for ALICE_PUB, and analogous checks for BOB_PUB
and ADMIN_PUB) so the script exits non‑zero if funding did not succeed.
| deploy_contracts() { | ||
| log_info "Deploying contracts..." | ||
|
|
||
| soroban network add --global sandbox local "${NETWORK_URL}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Avoid global config pollution; use sandbox-scoped network configuration.
The --global flag modifies the user's global Soroban configuration instead of scoping it to the sandbox. This violates the principle of isolated local development environments.
♻️ Proposed fix using environment-based configuration
Instead of soroban network add --global, either:
- Use a sandbox-specific config directory:
+ export SOROBAN_CONFIG_HOME="${PROJECT_ROOT}/.soroban-sandbox"
+ mkdir -p "${SOROBAN_CONFIG_HOME}"
- soroban network add --global sandbox local "${NETWORK_URL}"
+ soroban network add sandbox local "${NETWORK_URL}"- Or use
--network-passphraseand--rpc-urlflags directly in deployment commands to avoid network add entirely.
📝 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.
Suggested change
| soroban network add --global sandbox local "${NETWORK_URL}" | |
| export SOROBAN_CONFIG_HOME="${PROJECT_ROOT}/.soroban-sandbox" | |
| mkdir -p "${SOROBAN_CONFIG_HOME}" | |
| soroban network add sandbox local "${NETWORK_URL}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sandbox.sh` at line 163, Replace the global Soroban network
registration so the sandbox does not mutate the user's global config: stop using
the `--global` flag in the `soroban network add --global sandbox local
"${NETWORK_URL}"` invocation and instead either (A) scope the add to a
sandbox-specific config dir by running the add under a sandbox config
environment (e.g. set `SOROBAN_CONFIG_DIR="${SANDBOX_CONFIG_DIR}"` and run
`soroban network add sandbox local "${NETWORK_URL}"`) or (B) avoid `soroban
network add` entirely and pass network parameters directly to deployment
commands (use `--network-passphrase` and `--rpc-url` on `soroban` commands like
`soroban contract deploy`) so the script uses `NETWORK_URL` without altering
global state.
|
|
||
| { | ||
| echo "# SkillSphere Sandbox Environment" | ||
| echo "# Generated on $(date -Iseconds)" |
There was a problem hiding this comment.
Replace GNU-specific date flag for macOS compatibility.
The -Iseconds flag is GNU-specific and will fail on macOS/BSD systems. Use a portable alternative.
🐛 Proposed fix for cross-platform compatibility
- echo "# Generated on $(date -Iseconds)"
+ echo "# Generated on $(date -u +"%Y-%m-%dT%H:%M:%S%z")"📝 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.
Suggested change
| echo "# Generated on $(date -Iseconds)" | |
| echo "# Generated on $(date -u +"%Y-%m-%dT%H:%M:%S%z")" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sandbox.sh` at line 196, The echo currently uses the GNU-only flag
"date -Iseconds" which breaks on macOS/BSD; update the echo line that contains
"$(date -Iseconds)" to generate an ISO8601/UTC timestamp with portable date
format specifiers (use date with explicit %Y-%m-%dT%H:%M:%S and a UTC indicator)
so the script works on both GNU and BSD/macOS systems.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Developers can now spin up a fully configured local Soroban environment,
deploy all contracts, and get funded test wallets with a single command.
Changes
scripts/sandbox.sh— full sandbox bootstrap script:soroban,stellar-cli,dockerstellar/quickstartDocker container.env.sandboxwith contract IDs and wallet keysMakefile— addedsandbox,sandbox-stop,sandbox-logstargetsAcceptance Criteria
make sandboxcommand works end-to-endTesting
Tested locally against a clean Docker environment.
Run
make sandboxand verify.env.sandboxis writtenand all contract IDs are populated.
Notes
.env.sandboxis gitignored — add it to.gitignoreif not already present.Fixed wallet seeds ensure reproducible addresses across sandbox restarts.
Closes #210
Issue #215 — KYC/KYB Hooks
Commit Message:
feat(identity): add KYC/KYB verification hooks with oracle pattern #215
PR Title:
feat: KYC/KYB Integration Hooks — admin flag + oracle verify (#215)
PR Description:
markdown## Summary
Adds optional, jurisdiction-safe KYC hooks to the identity contract.
Admins can flag accounts as requiring KYC; only an authorized oracle
can flip the verified status.
Changes
contracts/src/identity.rs:KycStatusenum:NotRequired | Required | Verified | Rejectedadmin_set_kyc_required()— admin-only, emitskyc_requiredeventoracle_verify_kyc()— oracle-only, approves or rejects,emits
kyc_status_updatedeventrequire_kyc_if_needed()— reusable guard for other contract functionsIdentityErrorvariants:NotAdmin,NotOracle,KycRequired,AccountNotFound#[cfg(test)]unit tests for status transitionsAcceptance Criteria
KYC_REQUIREDKYC_VERIFIEDTesting
Unit tests cover:
admin_set_kyc_requiredoracle_verify_kycNotes
KYC is entirely optional per account — accounts without the flag
set remain
NotRequiredand passrequire_kyc_if_neededfreely.No breaking changes to existing identity logic.
Closes #215
Issue #216 — ZK Identity Design
Commit Message:
docs(identity): add ZK proof design document for privacy-preserving identity #216
PR Title:
docs: Privacy-Preserving ZK Identity Design Document (#216)
PR Description:
markdown## Summary
Research and design document for integrating zero-knowledge proofs
into SkillSphere's identity layer, enabling users to prove verification
without revealing their actual identity.
Changes
docs/ZK_IDENTITY.mdcovering:Acceptance Criteria
Notes
No contract code changes in this PR — this is a pure research/design
deliverable. Implementation will follow in a subsequent issue once
Soroban's ZK host function support matures or the oracle approach
is approved by the team.
Closes #216
Batch PR Option
If you want to ship all four as one PR instead:
PR Title:
feat: cross-chain bridge, local sandbox, KYC hooks, ZK identity design (#210 #215 #216 #217)
Commit sequence:
feat(dev): add local Soroban sandbox with make sandbox command #210
feat(identity): add KYC/KYB verification hooks with oracle pattern #215
docs(identity): add ZK proof design document for privacy-preserving identity #216
feat(bridge): add Axelar cross-chain bridge stubs and design doc #217
closes #217
closes #210
closes #215
closes #216
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores