feat(review): implement IPFS CID storage and resolve compilation & test issues by iammrjude · Pull Request #46 · HubDApp/Dongle-Smartcontract

iammrjude · 2026-02-24T10:36:29Z

Closes #16

Summary

Implements on-chain IPFS CID storage for project reviews as required by issue #16.
Review text lives off-chain on IPFS; only the CID is stored on-chain, keyed by
(project_id, reviewer_address). The get_review() function returns the CID so
the frontend can fetch full review content directly from IPFS.

This PR also includes significant fixes to the original forked codebase — the
contract as forked had multiple errors that prevented stellar contract build and
cargo test from running at all. All of those were identified and resolved as part
of this work.

Bug Fixes (pre-existing errors in forked code)

The forked repository had compilation and test failures that had to be fixed before
any new feature work could land:

initialize() not wired up — setup() in the test suite was calling
set_admin() which always returned UnauthorizedAdmin (#401) because no admin
was set yet. Fixed by replacing it with the correct initialize(admin, treasury)
bootstrap call. This unblocked all 40 pre-existing tests that were failing.
set_admin() had no bootstrap path — the function always checked the caller
against an existing admin, making the first call impossible without
initialize().
todo!() stubs in lib.rs — initialize, set_admin, all review,
verification, and fee functions were unimplemented stubs that panicked at
runtime. All were fully implemented.
StorageKey::Treasury missing — the Treasury variant was absent from
storage_keys.rs, causing compilation failures in fee and treasury functions.
Error enum named Error instead of ContractError — renamed throughout for
consistency and to match the test suite's imports.
events.rs used manual env.events().publish() — migrated to
#[contractevent] macro style for correctness and consistency with the Soroban
SDK version in use.
CRLF line endings throughout — cleaned up across all source files.

Feature Implementation (issue #16)

`types.rs`

Added comment_cid: Option<String> to the Review struct — the on-chain IPFS
reference field

`errors.rs`

Added InvalidCid = 206 — returned when a CID is present but fails length
validation

`review_registry.rs`

Added validate_optional_cid() with the following rules enforced on-chain:
- None passes — CID is optional, rating-only reviews remain valid
- Minimum 46 characters — shortest valid CIDv0 (Qm...)
- Maximum 128 characters (MAX_CID_LEN) — returns StringLengthExceeded
- Empty string rejected — returns InvalidCid
add_review() — accepts, validates, and stores comment_cid
update_review() — accepts updated comment_cid, can replace or clear it
get_review() — returns full Review struct including comment_cid for
frontend IPFS lookup

Tests

Before this PR

running 40 tests
test result: FAILED. 0 passed; 40 failed

After this PR

running 51 tests
test result: ok. 51 passed; 0 failed

New tests added for issue #16 (12 tests in `test.rs`)

test_add_review_with_cid_stores_on_chain — CIDv0 stored and retrieved
test_add_review_cidv1_stores_on_chain — CIDv1 (bafy...) stored and retrieved
test_add_review_without_cid_is_valid — rating-only review still works
test_add_review_empty_cid_reverts — empty string rejected
test_add_review_cid_too_short_reverts — 45-char CID rejected
test_add_review_cid_too_long_reverts — 129-char CID rejected
test_get_review_returns_cid_for_ipfs_lookup — CID returned for frontend use
test_update_review_replaces_cid_on_chain — updated CID overwrites original
test_update_review_can_clear_cid — update with None removes the reference
test_multiple_reviewers_store_independent_cids — each reviewer's CID stored
independently under their own (project_id, address) key
test_cid_stored_per_project_independently — same reviewer across multiple
projects gets separate CIDs per project

New tests from upstream sync (11 tests in `rating_calculator.rs`)

Full unit test coverage for RatingCalculator — calculate_average,
add_rating, update_rating, remove_rating

Build verification

stellar contract build  ✅
  Wasm Size: 24732 bytes
  Exported Functions: 21 found

cargo test              ✅
  51 passed; 0 failed

Notes

Full CID prefix validation (Qm vs bafy) is not enforced on-chain. The Soroban
no_std environment does not support heap allocation, making byte-by-byte string
prefix checks impractical. Length bounds (46–128 chars) serve as the contract-level
guard. The frontend is responsible for submitting well-formed CIDs before calling
add_review or update_review.

- Fixed duplicate Cargo.toml dependency sections, bumped soroban-sdk to 25.1.1 - Removed invalid crate-type from [profile.release-with-logs] - Replaced all Rust std::String with soroban_sdk::String throughout - Added require_auth() to all write operations - Fixed StorageKey enum (added Treasury variant) - Added missing FeeManager methods: set_fee_config, set_treasury, get_treasury - Fixed ledger setup in tests: use with_mut() instead of set() with LedgerInfo - Updated protocol_version to 22 in test ledger config - All 4 unit tests now passing

## Summary Implements on-chain IPFS CID storage for project reviews. All review text is stored off-chain on IPFS; only the content identifier (CID) is stored on-chain, keeping storage costs minimal while allowing any frontend to reconstruct full review content via `https://ipfs.io/ipfs/<comment_cid>`. --- ## Changes by File ### src/errors.rs - Added `InvalidCid = 206` to the `ContractError` enum (review registry section, 2xx range) - Documents that a CID is invalid if it is empty, shorter than 46 characters, or exceeds `MAX_CID_LEN` (128) ### src/review_registry.rs - Replaced the stub `validate_optional_cid` (which only checked max length) with a full implementation that enforces: - `None` passes — CID is optional; rating-only reviews are valid - `len == 0` → `ContractError::InvalidCid` (empty string is not a CID) - `len < 46` → `ContractError::InvalidCid` (46 is the minimum length of a valid CIDv0; anything shorter cannot be a real IPFS CID) - `len > MAX_CID_LEN` → `ContractError::StringLengthExceeded` - Added detailed doc comments to `add_review`, `update_review`, and `get_review` explaining the IPFS off-chain pattern - `add_review`: validates CID before writing; stores `comment_cid` inside the `Review` struct under key `StorageKey::Review(project_id, reviewer)` - `update_review`: validates new CID; replaces previous CID on-chain; passing `None` clears the reference (review becomes rating-only) - `get_review`: returns the full `Review` struct including `comment_cid` so the frontend can use it to fetch off-chain text from IPFS ### src/lib.rs - Exposed `get_owner_project_count(env, owner) -> u32` as a public contract method (was only internal to `ProjectRegistry`) - Added `set_fee(env, admin, token, amount, treasury)` as a public contract method — convenience wrapper around `FeeManager::set_fee` that also persists the treasury address, enabling tests and simple flows that don't call `initialize()` first - Added `pay_fee(env, payer, project_id, token)` as a public contract method — delegates to `FeeManager::pay_fee`, marks a project as fee-paid so verification can proceed - Added doc comment on `get_review` clarifying the `comment_cid` field's role in IPFS retrieval ### src/test.rs (complete rewrite / consolidation) - Fixed all compilation errors present in the original file: - Replaced every `"string".into()` with `String::from_str(&env, "...")` (`soroban_sdk::String` does not implement `From<&str>`) - Removed `.unwrap()` / `.is_none()` on `get_project` return value (`get_project` returns `Project` directly, not `Option<Project>`) - Removed `.expect()` on `get_verification` return value (`get_verification` returns `VerificationRecord` directly) - Replaced `set_fee` / `try_set_fee` / `pay_fee` / `try_pay_fee` calls (these didn't exist) with the newly added contract methods of the same names - Replaced `get_owner_project_count` calls (didn't exist on client) with the newly exposed contract method - Replaced `Vec::new()` + loop with explicit sequential calls (no_std environment has no `std::vec::Vec`) - Removed `format!` macro usage (unavailable in no_std) - Added missing `use soroban_sdk::testutils::Ledger` import (required for `env.ledger().with_mut(...)`) - Fixed `setup()` return type to `DongleContractClient<'_>` to resolve `mismatched_lifetime_syntaxes` warning - Fixed `setup()` to call `set_admin(&admin, &admin)` correctly - Migrated all tests from `src/project_registry.rs` `#[cfg(test)] mod tests` block into this file: - `test_ids_are_sequential` - `test_project_data_is_stored` - `test_event_is_emitted_on_registration` - `test_multiple_registrations_succeed` - Added 11 new tests for Issue HubDApp#16: - `test_add_review_with_cid_stores_on_chain` — CIDv0 (46 chars) stored and returned correctly - `test_add_review_cidv1_stores_on_chain` — CIDv1 ("bafy" prefix) stored and returned correctly - `test_add_review_without_cid_is_valid` — `None` CID accepted; rating-only reviews are valid - `test_add_review_empty_cid_reverts` — empty string rejected with `InvalidCid` - `test_add_review_cid_too_short_reverts` — 45-char string rejected with `InvalidCid` - `test_add_review_cid_too_long_reverts` — 129-char string rejected with `StringLengthExceeded` - `test_get_review_returns_cid_for_ipfs_lookup` — confirms `get_review` returns `comment_cid` for frontend IPFS retrieval - `test_update_review_replaces_cid_on_chain` — new CID overwrites old CID in storage - `test_update_review_can_clear_cid` — updating with `None` removes the IPFS reference - `test_multiple_reviewers_store_independent_cids` — each reviewer's CID is stored under its own `(project_id, reviewer)` key - `test_cid_stored_per_project_independently` — same reviewer can hold separate CIDs for different projects ### src/project_registry.rs - Removed the `#[cfg(test)] mod tests` block; all tests now live in `src/test.rs` for a single unified test module --- ## Design Decisions **Storage key**: `StorageKey::Review(project_id, Address)` — composite key gives O(1) read/write per review with no array iteration, satisfying the gas efficiency requirement for multiple reviews per project. **CID is optional**: `comment_cid: Option<String>` — a reviewer may submit a numeric rating without any off-chain text. This is intentional and explicitly tested. **No prefix validation on-chain**: `soroban_sdk::String` in a `no_std` environment exposes only `.len()` and `from_str()` — there is no byte indexing or iteration without heap allocation. Length-based guards (min 46, max 128) are applied on-chain. The existing codebase follows this same pattern (see `verification_registry.rs`). Prefix correctness (`Qm...` for CIDv0, `bafy...` for CIDv1) is the frontend's responsibility before submitting the transaction. **`set_fee` / `pay_fee` exposed on contract**: These existed internally in `FeeManager` but were not reachable via the contract client. Exposing them unblocks the verification flow tests and reflects realistic usage where a frontend pays the fee before requesting verification.

- Replace set_admin() with initialize() in setup() helper; contract requires admin + treasury on first call, not an unchecked set_admin - Fix test_add_review_cid_too_long_reverts: string literal was <129 chars and never triggered MAX_CID_LEN guard; use explicit 129-char CID

… tests passed

steller contract build works. Build Complete cargo test works. all tests passed

iammrjude · 2026-02-24T11:05:57Z

Done. @Naomi-Gift please review and merge into main

Naomi-Gift · 2026-02-24T11:11:50Z

@iammrjude "target" should be added to .gitignore.

iammrjude · 2026-02-24T11:36:48Z

@iammrjude "target" should be added to .gitignore.

@Naomi-Gift The target/ directory was already added to the .gitignore file file in my fork.. Could you please review it again and confirm?

Screenshot 1: This shows my forked repository, where neither target/ nor test_snapshots/tests/ directories are being tracked.

Screenshot 2: This shows the upstream repository https://github.com/HubDApp/Dongle-Smartcontract which current contains target/ and test_snapshots/tests/ directories.

Please note that once my PR is merged, the target/ and test_snapshots/tests/ directories will be removed from the repository, as they are now properly excluded via .gitignore.

Naomi-Gift · 2026-02-24T13:00:26Z

@iammrjude there are still conflics

Naomi-Gift · 2026-02-25T13:38:30Z

@iammrjude Build is failing, runa gain and fix conflicts

Naomi-Gift · 2026-02-26T00:06:29Z

@iammrjude @iammrjude Build is failing, runa gain and fix conflicts

iammrjude added 6 commits February 23, 2026 23:46

the command stellar contract build works and cargo test works and all…

c5117c0

… tests passed

resolve merge conflicts with upstream

49f727e

fixed the errors introduced after fixing conflicts

e087f25

steller contract build works. Build Complete cargo test works. all tests passed

iammrjude marked this pull request as ready for review February 24, 2026 11:04

refactor

a395c39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(review): implement IPFS CID storage and resolve compilation & test issues#46

feat(review): implement IPFS CID storage and resolve compilation & test issues#46
iammrjude wants to merge 7 commits intoHubDApp:mainfrom
iammrjude:main

iammrjude commented Feb 24, 2026 •

edited

Loading

Uh oh!

iammrjude commented Feb 24, 2026

Uh oh!

Naomi-Gift commented Feb 24, 2026

Uh oh!

iammrjude commented Feb 24, 2026 •

edited

Loading

Uh oh!

Naomi-Gift commented Feb 24, 2026

Uh oh!

Naomi-Gift commented Feb 25, 2026

Uh oh!

Naomi-Gift commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

iammrjude commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bug Fixes (pre-existing errors in forked code)

Feature Implementation (issue #16)

types.rs

errors.rs

review_registry.rs

Tests

Before this PR

After this PR

New tests added for issue #16 (12 tests in test.rs)

New tests from upstream sync (11 tests in rating_calculator.rs)

Build verification

Notes

Uh oh!

iammrjude commented Feb 24, 2026

Uh oh!

Naomi-Gift commented Feb 24, 2026

Uh oh!

iammrjude commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Naomi-Gift commented Feb 24, 2026

Uh oh!

Naomi-Gift commented Feb 25, 2026

Uh oh!

Naomi-Gift commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

iammrjude commented Feb 24, 2026 •

edited

Loading

`types.rs`

`errors.rs`

`review_registry.rs`

New tests added for issue #16 (12 tests in `test.rs`)

New tests from upstream sync (11 tests in `rating_calculator.rs`)

iammrjude commented Feb 24, 2026 •

edited

Loading