fix: resolve feature_flags/score/validation compile errors and CI test-masking bug#484
Open
RUKAYAT-CODER wants to merge 7 commits into
Open
fix: resolve feature_flags/score/validation compile errors and CI test-masking bug#484RUKAYAT-CODER wants to merge 7 commits into
RUKAYAT-CODER wants to merge 7 commits into
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cargo fmt --checkhas been failing onmainfor a while (pre-existingfeature_flags.rs/lib.rs/tokenization.rsformatting drift), which meant theclippystep inci.ymlnever actually got to run - it's gated behind fmt passing first. Separately,regression.ymlpipescargo test/gas benchmarks intoteewithoutset -o pipefail, so the step's exit code came fromtee(always 0) instead ofcargo test, silently masking real compile failures as green checks.Together, these two gaps meant the following real bugs in
contracts/teachlinkwere never caught:validation.rs:MAX_OPERATIONAL_TIMEOUTandMAX_TIME_SKEWwere each defined twice in the samepub mod configblock (identical values, just written two different ways - a copy/paste duplicate). Removed the second pair.score.rs: defined its own localScoreError/ScoreResult, which collided with the canonical#[contracterror]-annotated versions inerrors.rsthatscore.rsalso (redundantly) imported. The local versions didn't implementFrom<soroban_sdk::Error>, which is required for#[contractimpl]to build - that's the root cause of theE0277trait-bound errors. Removed the local duplicate;score.rsnow uses onlyerrors::ScoreError/ScoreResult.lib.rs: removed the now-danglinguse crate::score::ScoreError;(the type is already brought into scope via the existingpub use errors::{...}re-export).errors.rs:feature_flags.rsreferencedBridgeError::InvalidParameterandBridgeError::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 calledSymbol::to_string()(no inherent method under#![no_std]- only available via thealloc-gatedToStringtrait, which isn't in scope) andHash<N>::get()(doesn't exist onHash, only onBytes). Rewrote it to hash the user address string plus the flag name'sValpayload bytes, and to convert the resultingHash<32>toBytes(which does have.get()) before reading the bucket byte.regression.yml: addedset -o pipefailto the three steps that pipe intotee, so a real failure actually fails the job going forward.Note:
lib.rs's change here is narrowly scoped to theuse crate::score::ScoreError;removal - it doesn't touch the mod-ordering/pub useformatting 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 onlib.rsmostly a no-op on rebase.Test plan
cargo build --workspacecargo test --workspace --libcargo clippy --workspace --all-features -- -D warningsregression.yml's "Run unit tests" step now actually fails if a real compile error is (re-)introduced