Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions contracts/buyback/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use soroban_sdk::{
contract, contractimpl, contracttype, symbol_short, Address, BytesN, Env, Symbol, Vec,
};

use brain_storm_shared::access;
use brain_storm_shared::{access, math};

// =============================================================================
// Storage keys
Expand Down Expand Up @@ -377,7 +377,7 @@ impl BuybackContract {
if bst_price == 0 {
return 0;
}
(xlm_amount.checked_mul(1_000_000).unwrap_or(xlm_amount)) / bst_price
brain_storm_shared::math::checked_mul_div_i128(xlm_amount, 1_000_000, bst_price)
}

fn get_bst_price(env: &Env) -> i128 {
Expand Down
1 change: 1 addition & 0 deletions contracts/credential_metadata/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ edition = "2021"

[dependencies]
soroban-sdk = { version = "21.5.0", features = ["alloc"] }
brain-storm-shared = { path = "../shared", default-features = false }

[dev-dependencies]
soroban-sdk = { version = "21.5.0", features = ["testutils"] }
Expand Down
11 changes: 11 additions & 0 deletions contracts/credential_metadata/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
#![no_std]
// Test builds need std (panic::catch_unwind); runtime stays no_std.
#[cfg(test)]
extern crate std;

use soroban_sdk::{
contract, contractimpl, contracttype, symbol_short, Address, Bytes, BytesN, Env, String, Symbol,
};
Expand Down Expand Up @@ -120,6 +124,8 @@ impl CredentialMetadataContract {
) -> u32 {
admin.require_auth();
validation::validate_admin(&env, &admin);
// Shared metadata rules (#1170) — same helpers the NFT contract uses.
validation::validate_metadata_fields(&course_name, &grade, &ipfs_hash);

// Store credential metadata first
let metadata = MetadataRecord {
Expand Down Expand Up @@ -173,6 +179,8 @@ impl CredentialMetadataContract {
) {
admin.require_auth();
validation::validate_admin(&env, &admin);
// Shared metadata rules (#1170) — reject invalid fields before storing.
validation::validate_metadata_fields(&course_name, &grade, &ipfs_hash);

let metadata = MetadataRecord {
credential_id,
Expand Down Expand Up @@ -202,6 +210,9 @@ impl CredentialMetadataContract {
validation::validate_admin(&env, &admin);

let mut metadata: MetadataRecord = validation::get_metadata_or_panic(&env, credential_id);
// Shared metadata rules (#1170) — the new values must be valid too, and
// the stored content pointer an update leaves untouched must still hold.
validation::validate_metadata_fields(&course_name, &grade, &metadata.ipfs_hash);

let history_count: u32 = env
.storage()
Expand Down
4 changes: 2 additions & 2 deletions contracts/credential_metadata/src/linkage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ mod tests {
let env = Env::default();
env.mock_all_auths();
let admin = Address::generate(&env);
let result = std::panic::catch_unwind(|| {
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
unlink(&env, &admin, 1);
});
}));
assert!(result.is_err());
}
}
50 changes: 44 additions & 6 deletions contracts/credential_metadata/src/validation.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
//! Credential metadata validation module
//! Centralizes common validation logic used across credential operations

use soroban_sdk::{Address, Env};
use soroban_sdk::{Address, Env, String};

use brain_storm_shared::validation as shared;

use crate::DataKey;

/// Validates that the caller is the contract admin
Expand Down Expand Up @@ -49,11 +52,46 @@ pub fn is_renewable(env: &Env, credential_id: u64, grace_period: u64) -> bool {
}
}

/// Non-panicking validity check for a stored credential (issue #1170).
///
/// Returns `false` when no record exists for `credential_id` (never panics on
/// a nonexistent credential), and otherwise validates every field of the
/// record the contract actually stores:
///
/// * `course_name` / `grade` — the same shared rules the write paths enforce,
/// * `ipfs_hash` — must be present (it is the record's only content pointer),
/// * `completion_date` / `expiry_timestamp` — a credential must not expire
/// before it was completed.
///
/// The rules come from `brain_storm_shared::validation`, so the report and the
/// rejection messages can never diverge.
pub fn is_metadata_valid(env: &Env, credential_id: u64) -> bool {
match get_metadata_checked(env, credential_id) {
Some(record) => {
shared::is_valid_course_name(&record.course_name)
&& shared::is_valid_grade(&record.grade)
&& shared::is_non_empty_string(&record.ipfs_hash)
&& record.expiry_timestamp >= record.completion_date
}
None => false,
}
}

/// Rejects invalid metadata fields on every write path (issue #1170).
///
/// Delegates to the shared rules so `credential_metadata` and `nft` validate
/// course names identically, and adds the content pointer check that applies
/// to every record this contract stores.
pub fn validate_metadata_fields(course_name: &String, grade: &String, ipfs_hash: &String) {
shared::require_valid_course_name(course_name);
shared::require_valid_grade(grade);
shared::require_non_empty_string(ipfs_hash);
}

/// Validates timestamp is in the future
///
/// Thin wrapper over the shared rule (issue #1170): one implementation of
/// "must be in the future" for the whole workspace.
pub fn validate_future_timestamp(env: &Env, timestamp: u64) {
let current_time = env.ledger().timestamp();
assert!(
timestamp > current_time,
"Timestamp must be in the future"
);
shared::require_future_timestamp(env, timestamp);
}
174 changes: 172 additions & 2 deletions contracts/credential_metadata/src/validation_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use soroban_sdk::{testutils::Address as _, Address, Env, String};

use crate::{CredentialMetadataContract, CredentialMetadataContractClient, DataKey, MetadataRecord};
use crate::validation::{
metadata_exists, get_metadata_checked, get_metadata_or_panic, is_renewable,
validate_admin, validate_future_timestamp,
get_metadata_checked, get_metadata_or_panic, is_metadata_valid, is_renewable, metadata_exists,
validate_admin, validate_future_timestamp, validate_metadata_fields,
};

fn setup() -> (Env, CredentialMetadataContractClient<'static>, Address) {
Expand Down Expand Up @@ -156,3 +156,173 @@ fn test_is_expired_returns_false_for_far_future_expiry() {
store_sample(&env, &client, &admin, 10, 9_999_999_999);
assert!(!client.is_expired(&10));
}

// ── Metadata field validation (Issue #1170) ─────────────────────────────────
//
// Field rules come from `brain_storm_shared::validation`; this contract only
// decides *when* to apply them (every write path) and exposes a non-panicking
// `is_metadata_valid` for callers that must report instead of reject.

#[test]
fn test_is_metadata_valid_false_when_missing() {
let (env, client, _) = setup();
let valid = env.as_contract(&client.address, || is_metadata_valid(&env, 1234));
assert!(!valid);
}

#[test]
fn test_is_metadata_valid_true_for_stored_record() {
let (env, client, admin) = setup();
store_sample(&env, &client, &admin, 55, 9_999_999);
let valid = env.as_contract(&client.address, || is_metadata_valid(&env, 55));
assert!(valid);
}

/// A record exactly as the write paths store it.
fn valid_record(env: &Env, credential_id: u64) -> MetadataRecord {
MetadataRecord {
credential_id,
course_name: String::from_str(env, "Rust Fundamentals"),
completion_date: 1_000,
expiry_timestamp: 9_999_999,
grade: String::from_str(env, "A"),
ipfs_hash: String::from_str(env, "QmHash"),
}
}

/// Writes a record straight to storage, bypassing the write-path gate, so the
/// reader (`is_metadata_valid`) is what gets tested.
fn write_record(env: &Env, client: &CredentialMetadataContractClient, record: MetadataRecord) {
let id = record.credential_id;
env.as_contract(&client.address, || {
env.storage()
.persistent()
.set(&DataKey::Metadata(id), &record);
});
}

#[test]
fn test_is_metadata_valid_false_for_empty_ipfs_hash() {
let (env, client, _) = setup();
let mut record = valid_record(&env, 70);
record.ipfs_hash = String::from_str(&env, "");
write_record(&env, &client, record);
let valid = env.as_contract(&client.address, || is_metadata_valid(&env, 70));
assert!(!valid);
}

#[test]
fn test_is_metadata_valid_false_for_invalid_course_name() {
let (env, client, _) = setup();
let mut record = valid_record(&env, 71);
record.course_name = String::from_str(&env, "x");
write_record(&env, &client, record);
let valid = env.as_contract(&client.address, || is_metadata_valid(&env, 71));
assert!(!valid);
}

#[test]
fn test_is_metadata_valid_false_when_expiry_before_completion() {
let (env, client, _) = setup();
let mut record = valid_record(&env, 72);
record.expiry_timestamp = 999; // completion_date is 1_000
write_record(&env, &client, record);
let valid = env.as_contract(&client.address, || is_metadata_valid(&env, 72));
assert!(!valid);
}

#[test]
#[should_panic(expected = "Course name must be between 3 and 100 characters")]
fn test_store_metadata_rejects_empty_course_name() {
let (env, client, admin) = setup();
client.store_metadata(
&admin,
&1,
&String::from_str(&env, ""),
&1_000,
&9_999_999,
&String::from_str(&env, "A"),
&String::from_str(&env, "QmHash"),
);
}

#[test]
#[should_panic(expected = "Course name must be between 3 and 100 characters")]
fn test_store_metadata_rejects_overlong_course_name() {
let (env, client, admin) = setup();
let overlong = "X".repeat(101);
client.store_metadata(
&admin,
&1,
&String::from_str(&env, &overlong),
&1_000,
&9_999_999,
&String::from_str(&env, "A"),
&String::from_str(&env, "QmHash"),
);
}

#[test]
#[should_panic(expected = "Grade must be 20 characters or less")]
fn test_store_metadata_rejects_empty_grade() {
let (env, client, admin) = setup();
client.store_metadata(
&admin,
&1,
&String::from_str(&env, "Rust Fundamentals"),
&1_000,
&9_999_999,
&String::from_str(&env, ""),
&String::from_str(&env, "QmHash"),
);
}

#[test]
#[should_panic(expected = "Course name must be between 3 and 100 characters")]
fn test_update_metadata_rejects_invalid_course_name() {
let (env, client, admin) = setup();
store_sample(&env, &client, &admin, 66, 9_999_999);
client.update_metadata(
&admin,
&66,
&String::from_str(&env, "x"),
&String::from_str(&env, "B"),
);
}

#[test]
#[should_panic(expected = "String must not be empty")]
fn test_store_metadata_rejects_empty_ipfs_hash() {
let (env, client, admin) = setup();
client.store_metadata(
&admin,
&1,
&String::from_str(&env, "Rust Fundamentals"),
&1_000,
&9_999_999,
&String::from_str(&env, "A"),
&String::from_str(&env, ""),
);
}

#[test]
fn test_validate_metadata_fields_accepts_valid_values() {
let env = Env::default();
// No panic = accepted
validate_metadata_fields(
&String::from_str(&env, "Rust Fundamentals"),
&String::from_str(&env, "A"),
&String::from_str(&env, "QmHash"),
);
}

#[test]
#[should_panic(expected = "String must not be empty")]
fn test_validate_metadata_fields_rejects_missing_ipfs_hash() {
let env = Env::default();
validate_metadata_fields(
&String::from_str(&env, "Rust Fundamentals"),
&String::from_str(&env, "A"),
&String::from_str(&env, ""),
);
}
Loading
Loading