Skip to content

Feature/smc 1796 - #101

Merged
DylanVerstraete merged 32 commits into
mainfrom
feature/SMC-1796
Sep 2, 2026
Merged

Feature/smc 1796#101
DylanVerstraete merged 32 commits into
mainfrom
feature/SMC-1796

Conversation

@ohyung1287

Copy link
Copy Markdown
Contributor
  • Split the repo into bridge/, loan/, and shared/ so bridge and loan tutorials are separate.
  • Keep a simplified ASC bridge (local ASCBase + ASCMinter, direct execute) not the write-ability Outbox/Relayer/Inbox stack.
  • Document the no-relayer flow clearly: burn -> attest -> proof -> execute; offchain worker is optional UX only.
  • Add basic fund-movement validations: queryId replay protection, wrapped-token / source-loan emitter checks, receipt/event validation.
  • Reduce tutorial friction: stock minter path, reuse EVM_V1_DECODER_LIBRARY_ADDRESS across bridge → loan, yarn utils:check_setup, contributor deploy notes moved out of the main tutorial path.
  • Align env examples with CC3 Testnet; add yarn verify, forge unit tests, and CI hardening (stderr capture + forge tests).
  • Validations: Replay via processedQueries; mint only for registered origin tokens; loan fund/repay only from registered sourceLoanContract.
  • Tests / CI: Forge tests for burn-log parsing and loan source binding; workflows updated for new paths + 2>&1 stderr capture from main.

Comment thread bridge/contracts/CONTRIBUTOR_NOTES.md
Comment thread bridge/contracts/DEPLOYMENT.md
Comment thread bridge/bridge-offchain-worker/README.md
Comment thread bridge/contracts/sol/ASCMinter.sol
Comment thread bridge/contracts/sol/ASCMinter.sol
Comment thread loan/contracts/sol/ASCLoanManager.sol Outdated
Comment thread shared/check_setup.ts Outdated
Comment thread .github/actions/install-js-deps/action.yml Outdated
Comment thread .github/workflows/bridge-offchain-worker.yml Outdated

@DylanVerstraete DylanVerstraete left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this by running the contracts and the tests. Structure looks good — shared/ removes real duplication, and the enum change is applied consistently in both minter call paths, which was easy to get half-right. Two things I'd fix before merge.

1. The Mint enum shift breaks hello-bridge against the stock minter

MinterActions gains a None holder, so Mint moves 0 → 1, and the clients now send action = 1. But bridge/.env.example still points at the same pre-deployed minter as main:

ASC_MINTER_CONTRACT_ADDRESS="0x2Be9B8640ED32815d3B9e8C92AbcD3F15F07396f"

That contract was deployed from the old enum where Mint = 0. I ran hello-bridge end-to-end against that exact address on main yesterday — it sends action = 0 and mints fine — so that deployment maps 0 → Mint. Sending it 1 won't mint, and hello-bridge is the first tutorial anyone runs.

Either redeploy the stock minter and update the address, or keep Mint = 0.

(I couldn't confirm by eth_call — actions 0/1/2 all revert at "Merkle proof validation failed", so the dispatch is unreachable without a real proof. This is inference from a working run, not a direct probe.)

2. The five security tests pass with the security check deleted

I removed the production guard entirely:

// require(whitelistedEmitters[emitter], "Emitter not whitelisted");   ← deleted

and re-ran forge test --root bridge:

[PASS] testEmitterWhitelist_rejectsUnregisteredEmitter()
[PASS] testEmitterWhitelist_acceptsAfterWrapOriginToken()
[PASS] testEmitterWhitelist_rejectsSpoofedEmitterAfterWrap()
[PASS] testQueryDedupe_rejectsProcessedQueryId()
[PASS] testQueryDedupe_acceptsFreshQueryId()
Suite result: ok. 5 passed; 0 failed

exposeRequireWhitelistedEmitter and exposeRequireFreshQuery reimplement the guards inside the harness, so the tests assert against a require the test file wrote rather than the contract's.

Worth saying: the protection itself is real — ASCBase does implement processedQueries. And the fix pattern is already in this PR: exposeProcessBurnLogs and the whole ASCLoanManagerHarness delegate to the real internals, so those tests are genuine. Just needs the same treatment.

Minor while you're in there: whitelistedEmitters is redundant — wrapOriginToken always sets it alongside wrappedTokens and nothing unsets either, so the existing wrappedTokens[...] != address(0) check already gave the same guarantee.

3. Heads up on overlap with #103 / #104 / #105

This deletes every file those three PRs touch, and six of the fixes get carried forward as bugs into the new paths:

  • hello-bridge: "Creditcoin Testnet v2" (wrong network — the faucet is 24h rate limited, so this costs a day)
  • hello-bridge: "100 CTC ≈ 9 oracle queries" (measured 0.000197 CTC/query)
  • custom-contracts-bridging: "1000 TEST" — TestERC20 mints 1,000,000
  • loan: Mintable (TEST) — the token is named Burn Test
  • loan: stale Note loan N repayment log sample
  • loan/scripts/register.ts: <IntereseBasisPoints> typo

The DEPLOY.md and offchain-worker ones resolve themselves since those files are deleted. Simplest is to merge the three small ones first and rebase this, or merge this first and I'll re-apply the six at the new paths — happy either way, just not in parallel.

ohyung1287 and others added 17 commits September 1, 2026 09:57
MegaLinter lychee returns 403 for npm package pages and the Sepolia
faucet link in CI; escape the npm pattern and add the faucet URL.
Prettier had reordered ABI JSON keys; restore abi-creator.sh format, enforce LF line endings for ABI JSON, and exclude generated ABI dirs from Prettier.
Wait for source-chain burn receipts before proof generation, and restore anchored lychee patterns for npm, docs.attestcoin.org, and the Google faucet.
Remove SPELL_LYCHEE_ARGUMENTS (lychee was parsing exclude regex as URLs in .mega-linter.yml) and rely on .lycheeignore for npm, docs.attestcoin.org, faucet, and gitignored .env file links.
Remove the if: false gate so loan-flow runs after the other devnet examples, and clone forge-std in the reusable workflow for loan contract deploys.
Fund the deployer with 0.01 ETH (matching other devnet examples), deploy TestERC20 from the shared CI wallet, and keep AuxiliaryLoanContract on the loan owner key with explicit forge error handling.
…n CI.

Re-check Creditcoin loan status after attestation waits, skip worker fund/repay proof submission when scripts handle proofs, and sync worker tracker from chain.
Register source-chain funding in register_loan for CI, poll Creditcoin loan status instead of grepping script output, retry proof execute, and remove fragile formatted-balance checks.

@DylanVerstraete DylanVerstraete left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the reorg and the security-relevant substance rather than every line, since the example CI jobs execute the tutorials end to end and all pass (loan-flow included).

What I verified: query replay dedupe is real and regression-tested through a proper harness (Query already processed); mint is gated on the burn log's registered emitter, closing the spoofed-emitter class; the vendored ASCBase copy is gone in favor of the published @gluwa/asc-contracts import (the right fix for the #103-#105 overlap); and #108's .env ignore survives. Good to go.

@DylanVerstraete
DylanVerstraete merged commit 6668487 into main Sep 2, 2026
17 checks passed
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.

5 participants