-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Canvas v0.10 and federation readiness #8537
Conversation
e99a197
to
b25b5cc
Compare
2d042a4
to
d657837
Compare
757bfa7
to
e6cb834
Compare
Need to fix tests but otherwise this is ready for review/QA. |
3a02aa2
to
b78552e
Compare
@raykyri what does this PR do? where's the description of the goals of these changes (should be a ticket, ideally), and how we can confirm that these goals were satisfactorily met? |
Good point.. I was going to work on this on Sunday but lost track of time. Let me move this into a draft PR until I can fix that up (hopefully later today). |
d137786
to
7a7aa13
Compare
✅
|
…d verify comments, threads, commentReactions, threadReactions, request the latest clock when signing actions, fix substrate login upgrade prettier and prettier-plugin-organize-imports upgrade eslint-plugin-prettier to 5.2.1 fix tests clean up types and call signatures lint fixes fix merge conflicts lint fixes, formatting fixes remove enforce session keys flag always get new session when performing login add verified check to ThreadCard fix type fix delete reaction contract controllers clean up unused code fix reaction deletion, standardize on deleting reactions by comment_msg_id and thread_msg_id lint fixes clean up msg_id passing fix fix lint upgrade to 0.10.6 move canvas node global out of server.ts fix await, update action types in tests add types to modelUtils provide DIDs in model seeder test calls update pnpm-lock remove stray knowledge base file fix clock values in modelUtils, pass thread_msg_id / comment_msg_id as needed when calling APIs from modelUtils fix typecheck, standardize on camelcase for threadId in tests fix camelcase fix
4083cea
to
5ca68b3
Compare
QA matrix:
Session re-auth test:
Multichain test:
Other issues not for this PR:
|
Facing the same issue as earlier #8537 (comment)
Also got some auth issues with metamask/keplr (but they are also on master branch). |
This should have fixed both the createThread and auth issues. If you're a super admin, you should disable that flag before testing this PR, or otherwise Roger has a PR that will be merged into master soon to fix a potential issue with super admin tRPC requests. |
Update: Retested, Now all wallet logins work both inside and outside the community. But I am still facing this thread creation issue with all wallets. Also the API build was failing for me (and per @raykyri it might be the cause of the create thread issue)
will be attempting to fix that and update. Update: The build works for me now but still getting the thread creation issue Screen.Recording.2024-09-13.at.9.11.39.PM.mov |
packages/commonwealth/client/scripts/controllers/server/sessions.ts
Outdated
Show resolved
Hide resolved
…able, fix thread_msg_id on comments
… msg_ids for message dependencies
Only found 1 issue, the other things I tried were auth with all wallets both with/without community scope, thread/comment/reply/reaction creation/deletion/updation and all of those things worked.
Screen.Recording.2024-09-13.at.11.49.37.PM.movThanks for all the changes. cc: @raykyri |
Closes #8918. All tests pass, ready for QA and code review
This PR updates hicommonwealth/commonwealth from Canvas 0.8 to Canvas 0.10, and fully implements a federation node backed by a separate Postgres database.
When creating, editing, or deleting comments, threads, or reactions, it attempts to create the corresponding Canvas action and post it to the database. This may fail if a dependency of the current action wasn't posted to Canvas (e.g. commenting on a thread when the thread was not signed on Canvas); in that case the action will proceed without a signature.
Full Description of Changes
address
withdid
around the codebase. The DIDs are unchanged from CAIP-2 identifiers except withdid:pkh:
prefixed before them; did:pkh is just a wrapper around CAIP-2 but this gives us a better standardized way to refer to user accounts as DIDs.canvas_hash
tocanvas_msg_id
canvas_msg_id
is used when comments depend on threads, reactions depend on comments, etc.es2022
module format.Note this PR requires a local database called
federation
, or requiresprocess.env.FEDERATION_POSTGRES_DB_URL
to be set. To QA, you can create a local federation database the same way you initialized Postgres while setting up Commonwealth:Or in psql:
Deployment Plan
Description of Changes
thread
comment
reactThread
reactComment
msgid
in all canvas actions that reference a different model. Also ensure that server-side verification is implemented and working for all actions.createThread
updateThread
(limited server-side verification)deleteThread
createComment
updateComment
(limited server-side verification)deleteComment
reactThread
reactComment
unreactComment
unreactThread
thread.topic
/api/getClock
OR just include in app.statusauto-log-out user if all their session keys are logged out(Difficult to do this without either 1) introducing extra latency on page load (bad), 2) refactoring initAppState, or 3) making the code more complicated if the user's session keys are all logged out)Todos before deploying:
Outside the scope of this PR: