-
Notifications
You must be signed in to change notification settings - Fork 6
fix: inboxes and app keys #277
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
Conversation
fc20c53
to
427d174
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes inbox creation and app key consistency issues by correcting address property references and ensuring matching public/private key pairs. The changes address problems where apps were using inconsistent keys and incorrect account addresses during inbox operations.
- Updates property references from
identity.address
toidentity.accountAddress
for proper account identification - Ensures app public keys match their corresponding private keys instead of using mismatched generated keys
- Adds debugging information to help identify inbox creator validation issues
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
packages/hypergraph-react/src/hooks/useOwnSpaceInbox.ts | Updates identity property reference for author account address |
packages/hypergraph-react/src/hooks/useExternalSpaceInbox.ts | Updates identity property reference for author account address |
packages/hypergraph-react/src/HypergraphAppContext.tsx | Fixes account address property and adds debugging for inbox creator validation |
apps/connect/src/routes/authenticate.tsx | Ensures consistent key usage between public and private keys with explanatory comments |
@@ -627,7 +627,7 @@ export function HypergraphAppProvider({ | |||
} | |||
const inboxCreator = Inboxes.recoverAccountInboxCreatorKey(response.inbox); | |||
if (inboxCreator !== identity.signaturePublicKey) { | |||
console.error('Invalid inbox creator', response.inbox); | |||
console.error('Invalid inbox creator', response.inbox, inboxCreator, identity.signaturePublicKey); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider using a more structured logging approach with clear labels for debugging values, such as: console.error('Invalid inbox creator:', { inbox: response.inbox, recovered: inboxCreator, expected: identity.signaturePublicKey })
console.error('Invalid inbox creator', response.inbox, inboxCreator, identity.signaturePublicKey); | |
console.error('Invalid inbox creator:', { | |
inbox: response.inbox, | |
recovered: inboxCreator, | |
expected: identity.signaturePublicKey, | |
}); |
Copilot uses AI. Check for mistakes.
@@ -424,6 +424,8 @@ function AuthenticateComponent() { | |||
body: JSON.stringify(message), | |||
}); | |||
const appIdentityResponse = await response.json(); | |||
// TODO: All apps are essentially using the same keys, we should change to using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comment spans multiple lines but only shows the first line in the diff. Consider adding a tracking issue reference or deadline to this TODO comment to ensure this architectural concern is addressed.
// TODO: All apps are essentially using the same keys, we should change to using | |
// TODO (Tracking issue: #1234): All apps are essentially using the same keys, we should change to using |
Copilot uses AI. Check for mistakes.
App identities aren't fully set up yet, so all apps are using the same keys for the moment.
This fixes inbox creation by ensuring the correct accountAddress is sent, and it also ensures the app's private and public keys match.