Feat/zero value parameter#597
Conversation
- Introduced `teye-common` crate with standardized error codes for Teye contracts. - Implemented `CommonError` enum for shared error handling across contracts. - Updated `vision_records` contract to use `CommonError` as `ContractError`. - Added documentation for error codes and migration notes to ensure clarity on changes.
…across-all-contracts
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ent, fuzz testing, mutation testing, and security scanning - Implemented `deploy-production.yml` for deploying contracts to the mainnet with input validation and artifact management. - Updated `deploy-testnet.yml` to streamline contract deployment to the Stellar testnet with improved input handling and Slack notifications. - Enhanced `fuzz.yml` to include matrix testing for multiple fuzz targets and added artifact upload for crashes. - Revamped `mutation.yml` to run mutation tests across multiple contracts with caching and artifact uploads. - Improved `pre-commit.yml` and `quality.yml` for better concurrency management and added system dependencies installation. - Consolidated security checks in `security.yml`, including dependency audits, license checks, and secret scanning. - Removed redundant `test.yml` workflow and integrated its functionalities into other workflows. - Added `deny.toml` for cargo-deny configuration to manage advisories and licenses. - Introduced `run_coverage.sh` script for local code coverage execution with optional HTML report generation.
…mprehensive-CI/CD-pipeline # Conflicts: # .github/workflows/test.yml
- Updated function signatures in `consent.rs` for better formatting. - Added type aliases in `keys.rs` to disambiguate types. - Introduced `admin_tiers` module in `lib.rs`. - Added deprecation warnings in event files to track migration. - Simplified function signatures in `staking` and `vision_records` modules. - Cleaned up test files for consistency and readability. - Adjusted formatting in `zk_verifier` module for clarity. - Refactored `zk_voting` module for improved code structure and readability.
…/github.com/TheDEV111/Teye-Contracts into Feat-Implement-comprehensive-CI/CD-pipeline
- Removed merge conflict markers and unnecessary commented code in `vision_records` tests. - Simplified assertions in `vision_records` tests for admin promotions and demotions. - Cleaned up `audit.rs`, `events.rs`, and `helpers.rs` in `zk_verifier` by removing merge conflict remnants. - Streamlined `lib.rs` in `zk_verifier`, removing unused constants and functions related to admin management. - Consolidated test cases in `test_zk_access.rs`, removing redundant tests and ensuring clarity in assertions. - Updated `zk_voting` to utilize the `ZkVerifier` trait and cleaned up proof generation functions in tests.
|
@TheDEV111 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
WalkthroughThis PR adds comprehensive CI/CD infrastructure via three GitHub Actions workflows for benchmarking, code coverage, and production deployment, integrates the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Rationale: Multiple heterogeneous concerns require separate reasoning—three distinct workflows demand validation of caching strategies, artifact handling, and deployment orchestration; the Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/check_storage_keys.sh (2)
16-30:⚠️ Potential issue | 🔴 CriticalCritical: Unresolved merge conflict markers break the script.
This shell script contains unresolved git merge conflict markers (
<<<<<<< HEAD,=======,>>>>>>> upstream/master) that will cause immediate syntax errors when executed. Shellcheck confirms this with SC1072/SC1073 errors.The conflict appears to introduce duplicate initialization code (lines 19-27 re-declare
ROOT_DIR,CONTRACTS_DIR,EXIT_CODEand duplicate the loop header) that already exists at lines 6-13. The HEAD side is empty, suggesting the correct resolution is to remove the entire conflict block and keep only the original code.🔧 Proposed fix to remove the first conflict block
contract_name=$(basename "$contract") -<<<<<<< HEAD -======= - -ROOT_DIR=$(git rev-parse --show-toplevel 2>/dev/null || pwd) -CONTRACTS_DIR="$ROOT_DIR/contracts" -EXIT_CODE=0 - -echo "Checking for storage key collisions in $CONTRACTS_DIR..." - -for contract in "$CONTRACTS_DIR"/*/; do - if [ -d "$contract/src" ]; then - contract_name=$(basename "$contract") - - ->>>>>>> upstream/master # We want to find actual DEFINITIONS or unique usages that represent keys.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_storage_keys.sh` around lines 16 - 30, Remove the unresolved git merge conflict markers and duplicated initialization block: delete the lines containing "<<<<<<< HEAD", "=======", and ">>>>>>> upstream/master" and remove the duplicate declarations of ROOT_DIR, CONTRACTS_DIR, EXIT_CODE and the duplicated for-loop header so the script keeps the original single initialization and the single for contract in "$CONTRACTS_DIR"/*/ loop (look for symbols ROOT_DIR, CONTRACTS_DIR, EXIT_CODE, and the for contract ...; ensure only one set remains and the script has no conflict markers).
59-74:⚠️ Potential issue | 🔴 CriticalCritical: Second unresolved merge conflict block with duplicate logic.
Another merge conflict block duplicates the comment and
const_definitions/duplicatesvariable assignments that already exist at lines 53-57. This redundancy will cause script execution failures and potential logic errors if not resolved.🔧 Proposed fix to remove the second conflict block
duplicates=$(echo "$const_definitions" | uniq -d) -<<<<<<< HEAD -======= - - # For now, let's grep for all symbol_short! and filter out common false positives - # or just be more specific about what constitutes a "definition". - - # If the same symbol is used multiple times, it might be fine (e.g. get/set). - # But if it's defined as a CONST multiple times, that's a collision. - - const_definitions=$(grep -r "const .*Symbol = symbol_short!(" "$contract/src" | \ - sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ - sort) - - duplicates=$(echo "$const_definitions" | uniq -d) - ->>>>>>> upstream/master if [ -n "$duplicates" ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check_storage_keys.sh` around lines 59 - 74, There is a second unresolved merge-conflict block duplicating the earlier logic and comments (including the variables const_definitions and duplicates and the use of symbol_short!), so remove the duplicated conflict markers and the entire repeated block (the second const_definitions/duplicates assignment and surrounding comments), keeping the original single implementation of const_definitions and duplicates and ensuring no leftover <<<<<<< / ======= / >>>>>>> markers remain.
🧹 Nitpick comments (2)
contracts/events/tests/validation_test.rs (1)
79-81: Avoid coupling this case to validation order.The important behavior here is “rejects a multiply-invalid request.” Requiring
InvalidInputspecifically because the empty-string checks currently run before version validation makes the test brittle to harmless guard reordering.Possible simplification
- // Empty topic/hash check comes before version check - assert_eq!(result, Err(Ok(EventError::InvalidInput))); + assert!(matches!( + result, + Err(Ok(EventError::InvalidInput)) | Err(Ok(EventError::InvalidSchema)) + ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/events/tests/validation_test.rs` around lines 79 - 81, The test currently couples behavior to validation order by asserting the exact error variant EventError::InvalidInput returned from client.try_register_schema; instead relax the assertion to only verify the request is rejected (e.g., assert that result is an Err) so the test simply checks "rejects a multiply-invalid request" without depending on which guard runs first—locate the call to client.try_register_schema in validation_test.rs and replace the strict equality check against Err(Ok(EventError::InvalidInput)) with a generic rejection check (e.g., assert!(matches!(result, Err(_))) or result.is_err())..github/workflows/benchmark.yml (1)
87-88: The regression suite is executed twice.
contracts/zk_verifier/tests/bench_verify.rsalready contains theregression_*tests, so the unfiltered--test bench_verifystep runs them once and the filteredregressionstep reruns the same cases. If you want separate reporting, split the regression cases into a second test file; otherwise one invocation is enough.Also applies to: 90-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 87 - 88, The workflow currently runs the same tests twice because the "Run ZK benchmark tests" step executes cargo test -p zk_verifier --test bench_verify (which already includes regression_* tests in bench_verify.rs) and a later filtered "regression" step reruns those same tests; fix by either removing the unfiltered cargo invocation (keep the filtered regression run) or moving the regression_* tests out of bench_verify.rs into a separate test file (e.g., regression.rs) and update the cargo invocations so bench_verify runs only non-regression tests while the filtered regression step targets the new file; look for the "Run ZK benchmark tests" step, the cargo command cargo test -p zk_verifier --test bench_verify, the regression_* tests in bench_verify.rs and the later filtered regression step and adjust accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/coverage.yml:
- Around line 49-59: The workflow step running cargo tarpaulin can fail because
tee opens coverage/tarpaulin.log before tarpaulin creates the coverage
directory; update the "Generate coverage" step to ensure the coverage directory
exists before running cargo tarpaulin (e.g., create the directory with mkdir -p
coverage) so the subsequent cargo tarpaulin --output-dir coverage and tee
coverage/tarpaulin.log succeed; modify the step that runs the cargo tarpaulin
command referenced in the diff so the directory is created first.
In @.github/workflows/deploy-production.yml:
- Around line 119-120: The deployment step currently parses CONTRACT_ID from
OUTPUT but never fails if the DEPLOYMENT_CONTRACT_ID line is missing; update the
logic around CONTRACT_ID=$(echo "$OUTPUT" | sed -n
's/^DEPLOYMENT_CONTRACT_ID=//p') to detect an empty or unset CONTRACT_ID and
fail the job (non-zero exit) with a clear error message (and avoid writing an
empty contract_id to GITHUB_OUTPUT) so the workflow treats a missing
DEPLOYMENT_CONTRACT_ID emitted by scripts/deploy.sh as a failed release.
In `@contracts/events/tests/validation_test.rs`:
- Around line 427-433: Add a unit test that exercises the state-changing
source-registration path instead of only the read helper: using the existing
setup() harness generate a fresh Address with Address::generate(&env), submit
the actual registration transaction/entry point for sources (call the contract's
registration method — locate the function named something like register_source,
register_source_tx, or the client method that performs registration),
await/commit the transaction, then assert that
client.is_registered_source(&fresh) returns true; also include a
re-registration/duplicate attempt check if the registration entry point has
explicit error handling.
In `@deny.toml`:
- Around line 28-31: Resolve the merge conflict markers around the copyleft
policy in deny.toml by removing the conflict markers (<<<<<<< HEAD, =======,
>>>>>>> upstream/master) and keep the copyleft = "deny" setting; ensure the
resulting file contains a single copyleft entry (copyleft = "deny") with no
leftover merge markers so parsing of deny.toml succeeds.
- Around line 5-18: The deny.toml contains merge artifacts and misplaced keys:
remove the git conflict markers and the stray "upstream/master" text,
consolidate duplicate [licenses] sections into one, and move advisory-related
keys (vulnerability, unmaintained, yanked, notice) into a single [advisories]
section; ensure [licenses] contains license-related keys only (unlicensed, allow
list, copyleft, confidence-threshold) and remove duplicate copyleft entries so
the final file has one [advisories] block with
vulnerability/unmaintained/yanked/notice and one [licenses] block with
unlicensed/allow/copyleft/confidence-threshold.
---
Outside diff comments:
In `@scripts/check_storage_keys.sh`:
- Around line 16-30: Remove the unresolved git merge conflict markers and
duplicated initialization block: delete the lines containing "<<<<<<< HEAD",
"=======", and ">>>>>>> upstream/master" and remove the duplicate declarations
of ROOT_DIR, CONTRACTS_DIR, EXIT_CODE and the duplicated for-loop header so the
script keeps the original single initialization and the single for contract in
"$CONTRACTS_DIR"/*/ loop (look for symbols ROOT_DIR, CONTRACTS_DIR, EXIT_CODE,
and the for contract ...; ensure only one set remains and the script has no
conflict markers).
- Around line 59-74: There is a second unresolved merge-conflict block
duplicating the earlier logic and comments (including the variables
const_definitions and duplicates and the use of symbol_short!), so remove the
duplicated conflict markers and the entire repeated block (the second
const_definitions/duplicates assignment and surrounding comments), keeping the
original single implementation of const_definitions and duplicates and ensuring
no leftover <<<<<<< / ======= / >>>>>>> markers remain.
---
Nitpick comments:
In @.github/workflows/benchmark.yml:
- Around line 87-88: The workflow currently runs the same tests twice because
the "Run ZK benchmark tests" step executes cargo test -p zk_verifier --test
bench_verify (which already includes regression_* tests in bench_verify.rs) and
a later filtered "regression" step reruns those same tests; fix by either
removing the unfiltered cargo invocation (keep the filtered regression run) or
moving the regression_* tests out of bench_verify.rs into a separate test file
(e.g., regression.rs) and update the cargo invocations so bench_verify runs only
non-regression tests while the filtered regression step targets the new file;
look for the "Run ZK benchmark tests" step, the cargo command cargo test -p
zk_verifier --test bench_verify, the regression_* tests in bench_verify.rs and
the later filtered regression step and adjust accordingly.
In `@contracts/events/tests/validation_test.rs`:
- Around line 79-81: The test currently couples behavior to validation order by
asserting the exact error variant EventError::InvalidInput returned from
client.try_register_schema; instead relax the assertion to only verify the
request is rejected (e.g., assert that result is an Err) so the test simply
checks "rejects a multiply-invalid request" without depending on which guard
runs first—locate the call to client.try_register_schema in validation_test.rs
and replace the strict equality check against Err(Ok(EventError::InvalidInput))
with a generic rejection check (e.g., assert!(matches!(result, Err(_))) or
result.is_err()).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b0c3c8a-6ba9-4ac6-a88a-91a68ac485ed
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.github/workflows/benchmark.yml.github/workflows/coverage.yml.github/workflows/deploy-production.ymlCargo.tomlcontracts/common/src/admin_tiers.rscontracts/events/tests/validation_test.rscontracts/zk_verifier/src/audit.rsdeny.tomlscripts/check_storage_keys.sh
| - name: Generate coverage | ||
| run: | | ||
| cargo tarpaulin \ | ||
| --workspace \ | ||
| --out xml \ | ||
| --output-dir coverage \ | ||
| --timeout 600 \ | ||
| --exclude-files "tests/*" "fuzz/*" "contracts/benches/*" "example/*" \ | ||
| --engine llvm \ | ||
| --no-fail-fast \ | ||
| 2>&1 | tee coverage/tarpaulin.log |
There was a problem hiding this comment.
Create coverage/ before piping tarpaulin output.
tee coverage/tarpaulin.log opens the file before cargo tarpaulin --output-dir coverage gets a chance to create the directory, so this step will fail on a clean runner with “No such file or directory.”
Proposed fix
- name: Generate coverage
run: |
+ mkdir -p coverage
cargo tarpaulin \
--workspace \
--out xml \
--output-dir coverage \
--timeout 600 \
--exclude-files "tests/*" "fuzz/*" "contracts/benches/*" "example/*" \
--engine llvm \
--no-fail-fast \
2>&1 | tee coverage/tarpaulin.log📝 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.
| - name: Generate coverage | |
| run: | | |
| cargo tarpaulin \ | |
| --workspace \ | |
| --out xml \ | |
| --output-dir coverage \ | |
| --timeout 600 \ | |
| --exclude-files "tests/*" "fuzz/*" "contracts/benches/*" "example/*" \ | |
| --engine llvm \ | |
| --no-fail-fast \ | |
| 2>&1 | tee coverage/tarpaulin.log | |
| - name: Generate coverage | |
| run: | | |
| mkdir -p coverage | |
| cargo tarpaulin \ | |
| --workspace \ | |
| --out xml \ | |
| --output-dir coverage \ | |
| --timeout 600 \ | |
| --exclude-files "tests/*" "fuzz/*" "contracts/benches/*" "example/*" \ | |
| --engine llvm \ | |
| --no-fail-fast \ | |
| 2>&1 | tee coverage/tarpaulin.log |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/coverage.yml around lines 49 - 59, The workflow step
running cargo tarpaulin can fail because tee opens coverage/tarpaulin.log before
tarpaulin creates the coverage directory; update the "Generate coverage" step to
ensure the coverage directory exists before running cargo tarpaulin (e.g.,
create the directory with mkdir -p coverage) so the subsequent cargo tarpaulin
--output-dir coverage and tee coverage/tarpaulin.log succeed; modify the step
that runs the cargo tarpaulin command referenced in the diff so the directory is
created first.
| CONTRACT_ID=$(echo "$OUTPUT" | sed -n 's/^DEPLOYMENT_CONTRACT_ID=//p') | ||
| echo "contract_id=$CONTRACT_ID" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Fail the job when the contract ID cannot be parsed.
If scripts/deploy.sh exits 0 but stops emitting the DEPLOYMENT_CONTRACT_ID= line, this workflow still succeeds and publishes an empty deployment ID in the report. Treat a missing parse as a failed release.
Proposed fix
- CONTRACT_ID=$(echo "$OUTPUT" | sed -n 's/^DEPLOYMENT_CONTRACT_ID=//p')
- echo "contract_id=$CONTRACT_ID" >> "$GITHUB_OUTPUT"
+ CONTRACT_ID="$(printf '%s\n' "$OUTPUT" | sed -n 's/^DEPLOYMENT_CONTRACT_ID=//p' | tail -n1)"
+ if [ -z "$CONTRACT_ID" ]; then
+ echo "Failed to parse DEPLOYMENT_CONTRACT_ID from deploy output" >&2
+ exit 1
+ fi
+ echo "contract_id=$CONTRACT_ID" >> "$GITHUB_OUTPUT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-production.yml around lines 119 - 120, The
deployment step currently parses CONTRACT_ID from OUTPUT but never fails if the
DEPLOYMENT_CONTRACT_ID line is missing; update the logic around
CONTRACT_ID=$(echo "$OUTPUT" | sed -n 's/^DEPLOYMENT_CONTRACT_ID=//p') to detect
an empty or unset CONTRACT_ID and fail the job (non-zero exit) with a clear
error message (and avoid writing an empty contract_id to GITHUB_OUTPUT) so the
workflow treats a missing DEPLOYMENT_CONTRACT_ID emitted by scripts/deploy.sh as
a failed release.
| #[test] | ||
| fn test_is_registered_source_fresh_address_returns_false() { | ||
| let (env, client, _admin) = setup(); | ||
| let fresh = Address::generate(&env); | ||
|
|
||
| assert!(!client.is_registered_source(&fresh)); | ||
| } |
There was a problem hiding this comment.
This doesn't actually cover the source-registration mutation path.
Issue #353 calls out zero-value coverage for source registration, but this section only checks the read helper on a fresh address. A regression in the actual state-changing source-registration entry point would still slip through this suite.
If helpful, I can sketch the missing mutation test using the existing setup() harness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/events/tests/validation_test.rs` around lines 427 - 433, Add a unit
test that exercises the state-changing source-registration path instead of only
the read helper: using the existing setup() harness generate a fresh Address
with Address::generate(&env), submit the actual registration transaction/entry
point for sources (call the contract's registration method — locate the function
named something like register_source, register_source_tx, or the client method
that performs registration), await/commit the transaction, then assert that
client.is_registered_source(&fresh) returns true; also include a
re-registration/duplicate attempt check if the registration entry point has
explicit error handling.
| <<<<<<< HEAD | ||
| vulnerability = "deny" | ||
| unmaintained = "warn" | ||
| yanked = "warn" | ||
| notice = "warn" | ||
|
|
||
| [licenses] | ||
| unlicensed = "deny" | ||
| ======= | ||
| unmaintained = "all" | ||
| yanked = "warn" | ||
|
|
||
| [licenses] | ||
| >>>>>>> upstream/master |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the valid values for the unmaintained field in cargo-deny advisories configuration?
💡 Result:
The valid values for the unmaintained field in cargo-deny's [advisories] configuration are: "all", "workspace", "transitive", and "none".
Citations:
🏁 Script executed:
find . -name "deny.toml" -type fRepository: Stellar-Teye/Teye-Contracts
Length of output: 82
🏁 Script executed:
# If deny.toml exists, read it to see the actual merge conflict
if [ -f "deny.toml" ]; then
cat -n deny.toml | head -40
fiRepository: Stellar-Teye/Teye-Contracts
Length of output: 1054
Critical: Malformed TOML with duplicate sections and merge artifacts breaks cargo-deny configuration.
The deny.toml file contains parsing errors that will prevent cargo deny check from running. The merge conflict has been partially resolved, but the result is incorrectly structured:
- Duplicate
[licenses]section (lines 10 and 18) - Settings mixed across wrong sections:
unmaintained,vulnerability,yanked, andnoticeappear outside the[advisories]section - Stray
upstream/mastertext on line 19 (merge artifact) - Duplicate
copyleft = "deny"on lines 29–30 unmaintained = "all"on line 12 is in the[licenses]section instead of[advisories]where it belongs
Note: unmaintained = "all" is a valid cargo-deny value (valid options are "all", "workspace", "transitive", "none"), but only in the [advisories] section, not [licenses].
Fix by consolidating into proper sections:
[advisories]
vulnerability = "deny"
unmaintained = "warn"
yanked = "warn"
notice = "warn"
[licenses]
unlicensed = "deny"
allow = [
"MIT",
"Apache-2.0",
"BSD-2-Clause",
"BSD-3-Clause",
"ISC",
"Unicode-3.0",
"Unicode-DFS-2016",
]
copyleft = "deny"
confidence-threshold = 0.8🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deny.toml` around lines 5 - 18, The deny.toml contains merge artifacts and
misplaced keys: remove the git conflict markers and the stray "upstream/master"
text, consolidate duplicate [licenses] sections into one, and move
advisory-related keys (vulnerability, unmaintained, yanked, notice) into a
single [advisories] section; ensure [licenses] contains license-related keys
only (unlicensed, allow list, copyleft, confidence-threshold) and remove
duplicate copyleft entries so the final file has one [advisories] block with
vulnerability/unmaintained/yanked/notice and one [licenses] block with
unlicensed/allow/copyleft/confidence-threshold.
| <<<<<<< HEAD | ||
| copyleft = "deny" | ||
| ======= | ||
| >>>>>>> upstream/master |
There was a problem hiding this comment.
Critical: Additional merge conflict around copyleft setting.
Another unresolved conflict exists for the copyleft = "deny" license policy. This line from HEAD adds protection against copyleft-licensed dependencies, which is often desirable for proprietary or permissively-licensed smart contract projects.
🔧 Proposed fix — resolve by keeping the copyleft policy
]
-<<<<<<< HEAD
copyleft = "deny"
-=======
->>>>>>> upstream/master
confidence-threshold = 0.8📝 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.
| <<<<<<< HEAD | |
| copyleft = "deny" | |
| ======= | |
| >>>>>>> upstream/master | |
| ] | |
| copyleft = "deny" | |
| confidence-threshold = 0.8 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deny.toml` around lines 28 - 31, Resolve the merge conflict markers around
the copyleft policy in deny.toml by removing the conflict markers (<<<<<<< HEAD,
=======, >>>>>>> upstream/master) and keep the copyleft = "deny" setting; ensure
the resulting file contains a single copyleft entry (copyleft = "deny") with no
leftover merge markers so parsing of deny.toml succeeds.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check_storage_keys.sh`:
- Around line 30-42: The collision detection currently builds const_definitions
without excluding test files, causing test-only symbol consts to be counted in
duplicates; update the const_definitions pipeline (the command that produces
const_definitions) to apply the same test-file exclusions used for symbols (the
grep -v "_test.rs" and grep -v "test/" filters) before sed, or reuse the earlier
filtered pipeline for const_definitions so duplicates is computed only from
non-test sources.
- Line 50: The command assigning duplicates is using a truncated `uniq -` which
causes a parse error; update the pipeline that builds `duplicates` (currently
using `const_definitions`) to call `uniq` with a valid flag or no flag and
ensure input is sorted when using `uniq -d` (i.e., replace the broken `uniq -`
with a correct invocation such as plain `uniq` or `uniq -d` combined with
`sort`) so the `duplicates=$(echo "$const_definitions" | uniq -)` line no longer
fails.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d582da9-d90d-4296-9bfb-66b6347c562a
📒 Files selected for processing (3)
Cargo.tomldeny.tomlscripts/check_storage_keys.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- Cargo.toml
- deny.toml
| symbols=$(grep -r "symbol_short!(" "$contract/src" | \ | ||
| grep -v "_test.rs" | grep -v "test/" | \ | ||
| sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ | ||
| sort) | ||
|
|
||
| # If the same symbol is used multiple times, it might be fine (e.g. get/set). | ||
| # But if it's defined as a CONST multiple times, that's a collision. | ||
|
|
||
| const_definitions=$(grep -r "const .*Symbol = symbol_short!(" "$contract/src" | \ | ||
| sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ | ||
| sort) | ||
|
|
||
| duplicates=$(echo "$const_definitions" | uniq -d) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "Filter logic in scripts/check_storage_keys.sh:"
sed -n '30,42p' scripts/check_storage_keys.sh
echo
echo "Test-like paths under contracts/*/src:"
fd -t f '(_test\.rs|tests?\.rs)$' contracts | grep '/src/' || true
fd -t d 'tests?$' contracts | grep '/src/' || trueRepository: Stellar-Teye/Teye-Contracts
Length of output: 1396
Route the test-file filter into the actual collision detection.
Lines 30-33 apply _test.rs / test/ exclusions to the symbols variable, but lines 38-42 rebuild const_definitions without these filters. Since duplicates is derived from the unfiltered const_definitions set, test-only symbol definitions will be incorrectly flagged as collisions. The codebase contains multiple test files (keys_test.rs, test.rs, prescription_tests.rs, etc.) that would be affected.
Suggested fix
- const_definitions=$(grep -r "const .*Symbol = symbol_short!(" "$contract/src" | \
- sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \
+ const_definitions=$(grep -r "const .*Symbol = symbol_short!(" "$contract/src" | \
+ grep -v "_test.rs" | grep -v "test/" | \
+ sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \
sort)📝 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.
| symbols=$(grep -r "symbol_short!(" "$contract/src" | \ | |
| grep -v "_test.rs" | grep -v "test/" | \ | |
| sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ | |
| sort) | |
| # If the same symbol is used multiple times, it might be fine (e.g. get/set). | |
| # But if it's defined as a CONST multiple times, that's a collision. | |
| const_definitions=$(grep -r "const .*Symbol = symbol_short!(" "$contract/src" | \ | |
| sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ | |
| sort) | |
| duplicates=$(echo "$const_definitions" | uniq -d) | |
| symbols=$(grep -r "symbol_short!(" "$contract/src" | \ | |
| grep -v "_test.rs" | grep -v "test/" | \ | |
| sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ | |
| sort) | |
| # If the same symbol is used multiple times, it might be fine (e.g. get/set). | |
| # But if it's defined as a CONST multiple times, that's a collision. | |
| const_definitions=$(grep -r "const .*Symbol = symbol_short!(" "$contract/src" | \ | |
| grep -v "_test.rs" | grep -v "test/" | \ | |
| sed -n 's/.*symbol_short!("\([^"]*\)").*/\1/p' | \ | |
| sort) | |
| duplicates=$(echo "$const_definitions" | uniq -d) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check_storage_keys.sh` around lines 30 - 42, The collision detection
currently builds const_definitions without excluding test files, causing
test-only symbol consts to be counted in duplicates; update the
const_definitions pipeline (the command that produces const_definitions) to
apply the same test-file exclusions used for symbols (the grep -v "_test.rs" and
grep -v "test/" filters) before sed, or reuse the earlier filtered pipeline for
const_definitions so duplicates is computed only from non-test sources.
|
|
||
| duplicates=$(echo "$const_definitions" | uniq -d) | ||
|
|
||
| duplicates=$(echo "$const_definitions" | uniq - |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
echo "Affected lines:"
sed -n '46,52p' scripts/check_storage_keys.sh
echo
echo "Bash syntax check (should exit 0 after the fix):"
bash -n scripts/check_storage_keys.shRepository: Stellar-Teye/Teye-Contracts
Length of output: 646
🏁 Script executed:
#!/bin/bash
# Create a temporary copy and apply the fix
cp scripts/check_storage_keys.sh /tmp/check_storage_keys_fixed.sh
# Apply the suggested fix
sed -i 's/duplicates=$(echo "$const_definitions" | uniq -$/duplicates=$(echo "$const_definitions" | uniq -d)/' /tmp/check_storage_keys_fixed.sh
echo "Verifying the fix:"
echo "Line 50 after fix:"
sed -n '50p' /tmp/check_storage_keys_fixed.sh
echo
echo "Running bash syntax check on fixed file:"
bash -n /tmp/check_storage_keys_fixed.sh && echo "✓ Syntax check passed" || echo "✗ Syntax check failed"Repository: Stellar-Teye/Teye-Contracts
Length of output: 230
Fix the truncated uniq invocation on line 50.
Line 50 ends at uniq -, causing a bash parse error that prevents the entire script from executing. The storage-key guard cannot run until this syntax error is resolved.
Suggested fix
- duplicates=$(echo "$const_definitions" | uniq -
+ duplicates=$(echo "$const_definitions" | uniq -d)📝 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.
| duplicates=$(echo "$const_definitions" | uniq - | |
| duplicates=$(echo "$const_definitions" | uniq -d) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 50-50: Couldn't parse this command expansion. Fix to allow more checks.
(SC1073)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check_storage_keys.sh` at line 50, The command assigning duplicates
is using a truncated `uniq -` which causes a parse error; update the pipeline
that builds `duplicates` (currently using `const_definitions`) to call `uniq`
with a valid flag or no flag and ensure input is sorted when using `uniq -d`
(i.e., replace the broken `uniq -` with a correct invocation such as plain
`uniq` or `uniq -d` combined with `sort`) so the `duplicates=$(echo
"$const_definitions" | uniq -)` line no longer fails.
…111/Teye-Contracts into feat/zero-value-parameter # Conflicts: # .github/workflows/ci.yml
|
@walterthesmart the workflow checks show an error, but when clicked there's no error msg to tell where ther error is from, pls review |
|
@walterthesmart pls merge |
2 participants
closes #353
validation_test.rs. The file covers zero, empty string, empty vector, and zero-ID inputs across all critical state-modifying functions (schema registration, event publishing, subscriptions, consumer groups, webhooks, replay/compaction, checkpoints, dead letter queue, source registration, and uninitialized contract guards).
Summary by CodeRabbit
New Features
Tests