Fix merge conflicts in challenge-6-stable-coin - #378
Closed
0xquantum3labs wants to merge 13 commits into
Closed
Conversation
…s section (develop @ aef54d5)
…al_rules.md - packages/snfoundry/.gitattributes: contracts/** merge=ours was swallowing Scarb.toml/Scarb.lock version bumps from base-challenge-template (proven at merge fbc6c9a: 0.62.1 -> silently kept at 0.60.0). Narrow it by pinning Scarb.toml/Scarb.lock to merge=theirs *after* the wildcard, since .gitattributes resolves conflicts last-match-wins and the prior ordering let contracts/** override the existing Scarb.lock rule too. - sync_other_challenges.yaml: the Compatible-versions sync script captured the trailing "Make sure you have..." sentence in both the source-extracted versions_list and the separately-preserved requirements_link, duplicating it in the destination README on every sync run. Bound the source regex to just the bullet-list lines. - global_rules.md: verified superseded by AGENTS.md (same content plus SKILLS.md pointers and the correct @starknet-start/* package names that match packages/nextjs/package.json; global_rules.md still had the stale @starknet-react/* names) and removed it.
Two mistakes in the prior commit, both caught in review: 1. contracts/Scarb.lock merge=theirs was a dead rule from the start (proven with git check-attr against the unmodified base: resolves to merge=ours) because it sat before the old contracts/** wildcard, which last-match-wins silently overrode. Scarb.lock has never actually been merge=theirs in practice -- lock-file drift on challenge branches came from non-merge sync commits regenerating it, not from this driver. Fixed by keeping the Scarb.lock override, but only after removing the wildcard that clobbered it (see narrowing below), so it's live now, not dead. 2. Giving Scarb.toml merge=theirs was wrong: challenge branches carry their own legitimate dependencies base doesn't have (e.g. challenge-6 intentionally keeps openzeppelin_security for MyUSDStaking's ReentrancyGuard, commit c3a178d; challenge-3/5 have their own extra deps too). merge=theirs would silently delete those on the next sync -- trading the original silent-freeze bug for a silent-delete bug, still silent, still bad. Scarb.toml now has no merge driver at all: narrowing contracts/** down to contracts/src/** and contracts/tests/** means no pattern matches it, so it gets a normal 3-way merge -- real base/challenge edits collide as a genuine conflict and the sync workflow opens a PR for a human. Also fixed missing trailing newline at end of file.
req_match was still capturing the whole accumulated 'Make sure you have...' block, not one copy: with re.DOTALL and a '\n###' lookahead, the non-greedy match still had to consume every already-duplicated copy to reach the next heading, then that whole block got re-appended as requirements_link. Combined with the already-fixed source_match, this meant existing duplication (e.g. challenge-0's README, verified at 20 copies) never shrank back down -- only stopped growing further. Anchored req_match to a single line instead (^...$ with MULTILINE, no DOTALL), so it captures exactly one copy of the sentence regardless of how many accumulated duplicates are in old_content. Verified against the real challenge-0-simple-nft README (20 duplicate copies) with base's README as source, run twice with the exact script logic committed here: 20 -> 1 -> 1. Also independently verified the captain's second finding: the stale Compatible-versions numbers (Starknet-devnet v0.7.2 / Scarb v2.15.1 / Snforge v0.55.0) are wrong in both base-challenge-template's own README and upstream/develop's README, and mismatch both repos' .tool-versions (scarb 2.20.0 / starknet-foundry 0.62.1 / starknet-devnet 0.9.1). This is a data bug in upstream's source-of-truth README, not something this sync script or PR can fix -- the script faithfully copies whatever source says.
Scarb.toml was bumped to openzeppelin_token/access >=3.0.0 and openzeppelin_interfaces >=2.0.0 in 11c01a2 (2026-05-14), but your_contract.cairo was never migrated alongside it. In OpenZeppelin Cairo 3.0 the ERC20 dispatchers moved out of openzeppelin_token into the separate openzeppelin_interfaces crate, so `scarb build` has failed on the default branch of a public repo ever since. This mirrors upstream scaffold-stark-2 commit d589c42 exactly. The remaining CI errors (ambiguous TryInto E2313 at :104, and the two unused-variable warnings at :80/:104) were cascades of this single unresolved import: with IERC20Dispatcher resolving to <missing>, the type of strk_contract_address could not be inferred, leaving try_into's target type unknown. All four resolve with this one line; no type annotations were added. scaffold-stark-2's .github/sync-speedrun-filter-rules excludes your_contract.cairo from the rsync (each challenge owns its copy) but not Scarb.toml, so the sync delivered the manifest half of d589c42 and structurally could not deliver the code half. No sync can fix this.
…caffold-Stark#648) your_contract.cairo is excluded from the rsync-based sync pipeline, so upstream's fix from ~11 months ago (scaffold-stark-2 PR Scaffold-Stark#648, commit 6037c2b, "fix: your contract set_greeting not updating premium") could never reach this repo automatically. Ported it manually here: - Guard the transfer_from + premium.write(true) block with `if amount_strk > 0`, so Option::Some(0) no longer performs a transfer_from/premium write it shouldn't. - Emit `premium: self.premium.read()` instead of the hardcoded `premium: true`, so the event reflects actual storage state. Both hunks must land together: the event fix alone would only turn a hardcoded lie into an honestly-reported wrong value, since premium.write(true) still fired unconditionally for Some(0) and premium.read() would still return true.
yarn.lock and package.json live at the repo root, outside the packages/nextjs and packages/snfoundry path filters, so pushes that only touch the lockfile never trigger CI. This let 4 yarn.lock PRs merge without a single CI run, leaving the status dashboard stale while the CI itself runs `yarn install --immutable`, which fails on any lockfile drift.
.tool-versions pins the scarb/starknet-foundry/devnet versions the job installs, and the "Check starknet-foundry version sync" step diffs it against Scarb.lock. Without it in the path filter, a .tool-versions bump would skip the exact check meant to catch a mismatch.
…s section (develop @ 0cb6276)
…s section (develop @ 94cb14f)
…s section (develop @ 6e4e8c8)
…oid unexpected conflicts
…enge-6-stable-coin
Collaborator
|
Superseded. Resolved directly on Squashing is why this conflict returned on three consecutive sync waves: a squash never advances the merge-base, so git kept re-diffing challenge-6 from an eleven-commit-old point ( The merge-base is now Tracked in #375. |
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.
This PR was automatically created to resolve merge conflicts between base-challenge-template and challenge-6-stable-coin.