Skip to content

fix: persist freelance escrow job state - #209

Open
playmaker410 wants to merge 1 commit into
drydocs:mainfrom
playmaker410:fix/jobs-state-persistence
Open

fix: persist freelance escrow job state#209
playmaker410 wants to merge 1 commit into
drydocs:mainfrom
playmaker410:fix/jobs-state-persistence

Conversation

@playmaker410

Copy link
Copy Markdown

Summary

Persist the freelance-escrow demo's jobs and milestone activity in a namespaced, versioned localStorage entry so created jobs and their on-chain assertion mappings survive page reloads.

The provider now restores validated data synchronously during state initialization, falls back to the seed jobs when storage is missing, malformed, or unavailable, and writes every jobs update without letting storage failures break the in-memory experience. Because on-chain reconciliation is already available, restored milestones with assertion IDs are reconciled before child UI renders when an authorized wallet is available.

Closes #148

Test plan

  • cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test pass locally (Rust sources are unchanged; these are left to the required CI job)
  • CONTRACT.md updated if the public interface changed (not applicable; no contract interface changes)
  • scripts/testnet-smoke.sh run against testnet, if this changes contract behavior in a way that affects the deployed flow (not applicable; no contract behavior changes)
  • pnpm install --frozen-lockfile, pnpm lint, and pnpm build pass for demos/freelance-escrow with CI's pinned pnpm 10
  • pnpm install --frozen-lockfile and pnpm build pass for packages/tholos-sdk with CI's pinned pnpm 10
  • Manually created a job in the browser, confirmed it was stored under tholos.freelance-escrow.jobs.v1, reloaded, and confirmed the job and milestone were restored
  • Manually verified corrupted and missing localStorage data fall back to the three seed jobs without crashing

@collinsezedike collinsezedike 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.

The persistence idea itself is right. Four issues, all inline, one of them (the blank-page gate) is serious enough to block merge on its own.

);

if (!initialReconciliationComplete) {
return null;

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.

JobsProvider returns null for the whole subtree until wallet detection and reconciliation finish, but App.tsx mounts WalletButton, RoleSwitcher, and the job list as children of JobsProvider. Any reload where a restored job has a milestone with assertionId, essentially every returning user once this ships, blanks the entire page, including for users with no wallet installed at all, until every reconciliation read resolves. This should gate only the milestone data that needs it, not the whole app shell.

}

const parsed: unknown = JSON.parse(stored);
return Array.isArray(parsed) && parsed.every(isJob) ? parsed : seedJobs;

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.

parsed.every(isJob) discards the entire stored jobs array if even one job or milestone fails validation, instead of dropping just the invalid entry. A user with several valid jobs plus one corrupted or future-schema entry loses all of their real data on reload, replaced with the 3 seed jobs. Filter out invalid entries individually instead of failing the whole array.

const [jobs, setJobs] = useState<Job[]>(seedJobs);
const [jobs, setJobs] = useState<Job[]>(loadJobsFromStorage);
const reconcileTrackerRef = useRef<ReconcileTracker>({ counter: 0, applied: new Map() });
const needsInitialReconciliation = jobs.some((job) =>

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.

needsInitialReconciliation re-scans every job and milestone on every render, but its result is only ever consumed once by the lazy useState initializer below. Every subsequent setJobs call (createJob, submitMilestone, reconcileFromChain, etc.) re-runs this full scan for a value that's already been discarded. Compute it once via a ref or inside the lazy initializer itself.

})();
}

void initialReconciliationRef.current.finally(() => {

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.

This effect re-registers a new .finally() on the same shared promise every time jobs changes during reconciliation. While Promise.all reconciles N milestones, each of the N setJobs calls re-runs this effect; since initialReconciliationRef.current is already set it skips creating a new promise but still calls .finally() again, so N handler closures stack up on one promise and all fire when it settles. Register the callback once, not on every dependency change.

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.

[Feature] Persist freelance-escrow demo state across page reloads

2 participants