test: hackathon pillar — select_winners distribution + claim#46
test: hackathon pillar — select_winners distribution + claim#46real-venus wants to merge 1 commit into
Conversation
Add tests/hackathon_pillar.rs covering the Pillar::Hackathon paths: - create_event validation (Single release + future deadline; budget escrowed, fee taken at deposit) - submit open-submission model, deadline gate, re-submit, idempotency, withdraw - select_winners distribution: single-recipient sweep and multi-position split, each asserting recipient + fee-account deltas and profile credit/rep/earnings - select_winners rejections: empty set, position not in distribution, duplicate position, replay, missing event, completed event, owner-auth requirement - claim_milestone rejected for a Single-release hackathon Part of boundlessfi#25. Closes boundlessfi#34.
📝 WalkthroughWalkthroughAdds ChangesHackathon Pillar E2E Test Suite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/events/src/tests/hackathon_pillar.rs`:
- Around line 257-266: The test currently verifies that submitted_at remains
unchanged on re-submission, but without advancing the ledger time between the
first and second submission, the test cannot meaningfully detect if submitted_at
is incorrectly being overwritten with the current time. Advance the ledger time
between where first_time is captured (from the first get_submission call) and
the second ctx.events.submit call, so that if the implementation incorrectly
updates submitted_at during re-submission, the assertion that
second.submitted_at equals first_time would fail and catch the bug.
- Around line 182-184: The current assertions in the three tests use
assert!(res.is_err(), ...) which only verify that an error occurred but don't
validate the specific error variant, allowing unrelated failures to pass.
Replace each assertion with assert_eq!(res, Err(...)) to check for the exact
error variant: in create_rejects_multi_release_kind use assert_eq!(res,
Err(Error::InvalidReleaseKind)), in create_rejects_missing_deadline use
assert_eq!(res, Err(Error::DeadlineRequired)), and in
create_rejects_past_deadline use assert_eq!(res,
Err(Error::DeadlineMustBeFuture)). This ensures each test validates the specific
error expected from ctx.events.try_create_event() rather than just confirming
failure.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9f0eb1a-9755-46d9-8ebf-3426f6ef8f4e
📒 Files selected for processing (2)
contracts/events/src/tests/hackathon_pillar.rscontracts/events/src/tests/mod.rs
| let res = ctx.events.try_create_event(¶ms, &op); | ||
| assert!(res.is_err(), "hackathon must use Single release"); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "hackathon_pillar.rs" | head -5Repository: boundlessfi/boundless-contract
Length of output: 217
🏁 Script executed:
find . -type f -name "errors.rs" | grep -E "(events|contracts)" | head -10Repository: boundlessfi/boundless-contract
Length of output: 235
🏁 Script executed:
fd -t f "errors\." --path "*/events/*"Repository: boundlessfi/boundless-contract
Length of output: 402
🏁 Script executed:
cat -n ./contracts/events/src/tests/hackathon_pillar.rs | sed -n '175,235p'Repository: boundlessfi/boundless-contract
Length of output: 2874
🏁 Script executed:
cat -n ./contracts/events/src/errors.rsRepository: boundlessfi/boundless-contract
Length of output: 2765
🏁 Script executed:
rg -n "fn try_create_event" ./contracts/events/src/ --type rust -A 5Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
rg -n "try_create_event" ./contracts/events/src/tests/hackathon_pillar.rs --type rust -B 2 -A 10 | head -40Repository: boundlessfi/boundless-contract
Length of output: 1531
🏁 Script executed:
rg -n "impl.*Events" ./contracts/events/src/ --type rust | head -20Repository: boundlessfi/boundless-contract
Length of output: 223
🏁 Script executed:
rg -n "struct Events" ./contracts/events/src/ --type rustRepository: boundlessfi/boundless-contract
Length of output: 228
🏁 Script executed:
cat -n ./contracts/events/src/tests/hackathon_pillar.rs | sed -n '1,50p'Repository: boundlessfi/boundless-contract
Length of output: 2477
🏁 Script executed:
rg -n "try_create_event\|create_event" ./contracts/events/src/lib.rs --type rust -A 3 | head -40Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
ast-grep outline ./contracts/events/src/lib.rsRepository: boundlessfi/boundless-contract
Length of output: 1316
🏁 Script executed:
rg -n "pub fn create_event" ./contracts/events/src/ --type rust -A 10Repository: boundlessfi/boundless-contract
Length of output: 1580
🏁 Script executed:
cat -n ./contracts/events/src/lib.rs | sed -n '40,150p'Repository: boundlessfi/boundless-contract
Length of output: 4359
🏁 Script executed:
rg -n "EventsContractClient" ./contracts/events/src/tests/hackathon_pillar.rs --type rust -B 5 -A 15 | head -50Repository: boundlessfi/boundless-contract
Length of output: 1895
🏁 Script executed:
find ./contracts/events -name "*.rs" -exec grep -l "EventsContractClient" {} \;Repository: boundlessfi/boundless-contract
Length of output: 394
🏁 Script executed:
rg -n "fn setup" ./contracts/events/src/tests/hackathon_pillar.rs -A 50 | head -80Repository: boundlessfi/boundless-contract
Length of output: 1913
🏁 Script executed:
cat -n ./contracts/events/src/tests/common.rs | head -100Repository: boundlessfi/boundless-contract
Length of output: 2708
🏁 Script executed:
rg -n "\.try_" ./contracts/events/src/tests/hackathon_pillar.rs --type rust | head -20Repository: boundlessfi/boundless-contract
Length of output: 1144
🏁 Script executed:
rg -n "EventsContractClient\|try_create_event" ./contracts/events/src/tests/crowdfunding.rs --type rust -A 3 | head -40Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
rg -n "try_create_event\|try_submit\|try_select" ./contracts/events/src/tests/hackathon_pillar.rs --type rust -A 2 | head -100Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
cat -n ./contracts/events/src/tests/hackathon_pillar.rs | sed -n '275,315p'Repository: boundlessfi/boundless-contract
Length of output: 1850
🏁 Script executed:
rg -n "Result.*Error" ./contracts/events/src/lib.rs --type rust | head -20Repository: boundlessfi/boundless-contract
Length of output: 1418
🏁 Script executed:
rg -n "InvokeContractError\|Result.*Result" ./contracts/events/src/tests/ --type rust | head -20Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
python3 - <<'PY'
import subprocess
import re
# Search for try_ method usage patterns
result = subprocess.run(
['rg', '-n', 'let res = ctx.events.try_', './contracts/events/src/tests/hackathon_pillar.rs', '-A', '1'],
capture_output=True,
text=True
)
print("=== try_ method usages ===")
lines = result.stdout.split('\n')
for i, line in enumerate(lines[:30]):
print(line)
# Now let's look at assertion patterns
result2 = subprocess.run(
['rg', '-n', 'assert.*res', './contracts/events/src/tests/hackathon_pillar.rs', '-B', '1'],
capture_output=True,
text=True
)
print("\n=== Assertions on res ===")
lines2 = result2.stdout.split('\n')
for i, line in enumerate(lines2[:50]):
print(line)
PYRepository: boundlessfi/boundless-contract
Length of output: 3490
🏁 Script executed:
rg -n "InvokeContractError\|soroban_sdk" ./contracts/events/src/tests/hackathon_pillar.rs --type rust | head -30Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
grep -r "InvokeContractError" ./contracts/events/src/ --include="*.rs"Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
rg -n "use soroban_sdk" ./contracts/events/src/tests/hackathon_pillar.rs --type rust -A 5Repository: boundlessfi/boundless-contract
Length of output: 397
🏁 Script executed:
cat -n ./contracts/events/src/tests/hackathon_pillar.rs | sed -n '1,35p'Repository: boundlessfi/boundless-contract
Length of output: 1989
🏁 Script executed:
python3 - <<'PY'
# Look for soroban docs or SDK usage patterns
import subprocess
# Find what the try_create_event return type is by looking at actual assertions
result = subprocess.run(
['rg', '-n', 'res\\.', './contracts/events/src/tests/hackathon_pillar.rs', '--type', 'rust'],
capture_output=True,
text=True
)
print("=== How res is used after try_ calls ===")
lines = result.stdout.split('\n')
for line in lines[:50]:
if line.strip():
print(line)
PYRepository: boundlessfi/boundless-contract
Length of output: 1159
🏁 Script executed:
find ./contracts -name "*.rs" -path "*/tests/*" -exec grep -l "assert.*Error::" {} \;Repository: boundlessfi/boundless-contract
Length of output: 245
🏁 Script executed:
rg -n "assert_eq.*Error\|assert.*Error::" ./contracts/events/src/tests/ --type rust -B 2 -A 1 | head -50Repository: boundlessfi/boundless-contract
Length of output: 168
🏁 Script executed:
cat -n ./contracts/events/src/tests/admin.rs | head -150Repository: boundlessfi/boundless-contract
Length of output: 5738
🏁 Script executed:
rg -n "Err\(" ./contracts/events/src/tests/ --type rust -B 1 -A 1 | head -60Repository: boundlessfi/boundless-contract
Length of output: 168
Assert concrete error variants instead of only is_err() in create validation tests.
These checks can pass on unrelated failures, missing the PR's "each error variant" objective. However, the suggested assertion format in the original comment uses incorrect syntax (Err(Ok(...))) that would not compile. Use assert_eq!(res, Err(Error::InvalidReleaseKind)) instead, following the pattern used elsewhere in the test suite (see admin.rs for reference).
The three tests at lines 182–184, 203–205, and 226–228 should each assert their specific error variant:
create_rejects_multi_release_kind:assert_eq!(res, Err(Error::InvalidReleaseKind))create_rejects_missing_deadline:assert_eq!(res, Err(Error::DeadlineRequired))create_rejects_past_deadline:assert_eq!(res, Err(Error::DeadlineMustBeFuture))
🤖 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/events/src/tests/hackathon_pillar.rs` around lines 182 - 184, The
current assertions in the three tests use assert!(res.is_err(), ...) which only
verify that an error occurred but don't validate the specific error variant,
allowing unrelated failures to pass. Replace each assertion with assert_eq!(res,
Err(...)) to check for the exact error variant: in
create_rejects_multi_release_kind use assert_eq!(res,
Err(Error::InvalidReleaseKind)), in create_rejects_missing_deadline use
assert_eq!(res, Err(Error::DeadlineRequired)), and in
create_rejects_past_deadline use assert_eq!(res,
Err(Error::DeadlineMustBeFuture)). This ensures each test validates the specific
error expected from ctx.events.try_create_event() rather than just confirming
failure.
| let first_time = ctx.events.get_submission(&id, &ctx.applicant).submitted_at; | ||
|
|
||
| let uri_b = String::from_str(&ctx.env, "ipfs://Qm.../v2.json"); | ||
| let op_b = BytesN::random(&ctx.env); | ||
| ctx.events.submit(&id, &ctx.applicant, &uri_b, &op_b); | ||
|
|
||
| let second = ctx.events.get_submission(&id, &ctx.applicant); | ||
| assert_eq!(second.content_uri, uri_b); | ||
| assert_eq!(second.submitted_at, first_time); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Advance ledger time before re-submit to make the timestamp assertion meaningful.
Without a timestamp change between submissions, this test can still pass if submitted_at is incorrectly overwritten with current time on re-submit.
Suggested fix
let first_time = ctx.events.get_submission(&id, &ctx.applicant).submitted_at;
+ ctx.env.ledger().with_mut(|li| {
+ li.timestamp += 1;
+ });
+
let uri_b = String::from_str(&ctx.env, "ipfs://Qm.../v2.json");
let op_b = BytesN::random(&ctx.env);
ctx.events.submit(&id, &ctx.applicant, &uri_b, &op_b);📝 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.
| let first_time = ctx.events.get_submission(&id, &ctx.applicant).submitted_at; | |
| let uri_b = String::from_str(&ctx.env, "ipfs://Qm.../v2.json"); | |
| let op_b = BytesN::random(&ctx.env); | |
| ctx.events.submit(&id, &ctx.applicant, &uri_b, &op_b); | |
| let second = ctx.events.get_submission(&id, &ctx.applicant); | |
| assert_eq!(second.content_uri, uri_b); | |
| assert_eq!(second.submitted_at, first_time); | |
| } | |
| let first_time = ctx.events.get_submission(&id, &ctx.applicant).submitted_at; | |
| ctx.env.ledger().with_mut(|li| { | |
| li.timestamp += 1; | |
| }); | |
| let uri_b = String::from_str(&ctx.env, "ipfs://Qm.../v2.json"); | |
| let op_b = BytesN::random(&ctx.env); | |
| ctx.events.submit(&id, &ctx.applicant, &uri_b, &op_b); | |
| let second = ctx.events.get_submission(&id, &ctx.applicant); | |
| assert_eq!(second.content_uri, uri_b); | |
| assert_eq!(second.submitted_at, first_time); | |
| } |
🤖 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/events/src/tests/hackathon_pillar.rs` around lines 257 - 266, The
test currently verifies that submitted_at remains unchanged on re-submission,
but without advancing the ledger time between the first and second submission,
the test cannot meaningfully detect if submitted_at is incorrectly being
overwritten with the current time. Advance the ledger time between where
first_time is captured (from the first get_submission call) and the second
ctx.events.submit call, so that if the implementation incorrectly updates
submitted_at during re-submission, the assertion that second.submitted_at equals
first_time would fail and catch the bug.
test(events): hackathon pillar — select_winners distribution + claim
Closes #34.
What
Adds
src/tests/hackathon_pillar.rs(+ a one-linemod hackathon_pillar;insrc/tests/mod.rs) covering every in-scope function of thePillar::Hackathonpath. Tests run against a real
boundless-profiledeployment so thecross-contract credit / reputation / earnings side-effects of
select_winnersare exercised, not mocked.
Coverage (18 tests)
create_event / validate_create
ReleaseKind::Multisubmit (open-submission model)
submitted_at, updates the URIop_idreverts (idempotency)withdraw_submissionremoves the anchorselect_winners — distribution (happy paths)
Completed)Completed)select_winners — rejections / edges
WinnersAlreadySelectedop_idCompletedeventenv.auths()demands the owner, not the admin)Summary by CodeRabbit