Skip to content

fix: resolve feature_flags/score/validation compile errors and CI test-masking bug#484

Open
RUKAYAT-CODER wants to merge 7 commits into
mainfrom
fix/feature-flags-score-validation-compile-errors
Open

fix: resolve feature_flags/score/validation compile errors and CI test-masking bug#484
RUKAYAT-CODER wants to merge 7 commits into
mainfrom
fix/feature-flags-score-validation-compile-errors

Conversation

@RUKAYAT-CODER

Copy link
Copy Markdown
Contributor

Summary

cargo fmt --check has been failing on main for a while (pre-existing feature_flags.rs/lib.rs/tokenization.rs formatting drift), which meant the clippy step in ci.yml never actually got to run - it's gated behind fmt passing first. Separately, regression.yml pipes cargo test/gas benchmarks into tee without set -o pipefail, so the step's exit code came from tee (always 0) instead of cargo test, silently masking real compile failures as green checks.

Together, these two gaps meant the following real bugs in contracts/teachlink were never caught:

  • validation.rs: MAX_OPERATIONAL_TIMEOUT and MAX_TIME_SKEW were each defined twice in the same pub mod config block (identical values, just written two different ways - a copy/paste duplicate). Removed the second pair.
  • score.rs: defined its own local ScoreError/ScoreResult, which collided with the canonical #[contracterror]-annotated versions in errors.rs that score.rs also (redundantly) imported. The local versions didn't implement From<soroban_sdk::Error>, which is required for #[contractimpl] to build - that's the root cause of the E0277 trait-bound errors. Removed the local duplicate; score.rs now uses only errors::ScoreError/ScoreResult.
  • lib.rs: removed the now-dangling use crate::score::ScoreError; (the type is already brought into scope via the existing pub use errors::{...} re-export).
  • errors.rs: feature_flags.rs referenced BridgeError::InvalidParameter and BridgeError::NotFound, neither of which exists. Appended two new variants (InvalidParameter = 150, FeatureFlagNotFound = 151) per the file's documented "append only, never renumber" convention, and updated the code-range doc table (which was already missing 148-149).
  • feature_flags.rs: the rollout-percentage bucketing helper called Symbol::to_string() (no inherent method under #![no_std] - only available via the alloc-gated ToString trait, which isn't in scope) and Hash<N>::get() (doesn't exist on Hash, only on Bytes). Rewrote it to hash the user address string plus the flag name's Val payload bytes, and to convert the resulting Hash<32> to Bytes (which does have .get()) before reading the bucket byte.
  • regression.yml: added set -o pipefail to the three steps that pipe into tee, so a real failure actually fails the job going forward.

Note: lib.rs's change here is narrowly scoped to the use crate::score::ScoreError; removal - it doesn't touch the mod-ordering/pub use formatting also being fixed in #483, so there will be some overlap between the two PRs on that file; whichever merges first will make the other's formatting diff on lib.rs mostly a no-op on rebase.

Test plan

  • cargo build --workspace
  • cargo test --workspace --lib
  • cargo clippy --workspace --all-features -- -D warnings
  • Confirm regression.yml's "Run unit tests" step now actually fails if a real compile error is (re-)introduced

…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix_commit_msg.txt
…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix2_commit_msg.txt
…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix3_commit_msg.txt
…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix4_msg.txt
…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix5_msg.txt
…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix6_msg.txt
…ktop-teachLink-contract/bb6b2c0c-69b2-43d6-ba37-ba3f90e13957/scratchpad/fix7_msg.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant