-
Notifications
You must be signed in to change notification settings - Fork 74
Updating Base Account Reference section #150
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
✅ Heimdall Review Status
|
{ | ||
success: false, | ||
error: "Insufficient balance", | ||
amount: "10.50", | ||
to: "0x1234567890123456789012345678901234567890" | ||
} | ||
``` | ||
</ResponseExample> | ||
|
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.
@spencerstock has an ongoing PR to throw errors instead of returning typed responses base/account-sdk#71
Let's hold off on this section until the PR is resolved
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.
I'll merge this now and update when Spencer merges his PR
- Session expired | ||
|
||
Always wrap calls in a try-catch block: | ||
|
||
```typescript | ||
async function safeGetAccount() { | ||
try { | ||
const cryptoAccount = await getCryptoKeyAccount(); | ||
return cryptoAccount?.account; | ||
} catch (error) { | ||
console.error('Error getting crypto account:', error); | ||
|
||
if (error.message.includes('not initialized')) { | ||
// SDK not properly initialized | ||
throw new Error('Base Account SDK not initialized'); | ||
} else if (error.message.includes('permission denied')) { | ||
// User denied access | ||
throw new Error('User denied account access'); | ||
} else if (error.message.includes('session expired')) { | ||
// Session needs refresh | ||
throw new Error('Session expired, please reconnect'); | ||
} else { | ||
// Unknown error | ||
throw new Error('Failed to access account'); | ||
} | ||
} | ||
} | ||
``` | ||
|
||
## Real-time Account Updates | ||
|
||
```typescript | ||
import { getCryptoKeyAccount } from '@base-org/account'; | ||
|
||
class AccountMonitor { | ||
private account: WebAuthnAccount | LocalAccount | null = null; | ||
private listeners: ((account: any) => void)[] = []; | ||
private pollInterval: NodeJS.Timeout | null = null; | ||
|
||
async start() { | ||
// Initial check | ||
await this.checkAccount(); | ||
|
||
// Poll for changes every 5 seconds | ||
this.pollInterval = setInterval(() => { | ||
this.checkAccount(); | ||
}, 5000); | ||
} | ||
|
||
stop() { | ||
if (this.pollInterval) { | ||
clearInterval(this.pollInterval); | ||
this.pollInterval = null; | ||
} | ||
} | ||
|
||
private async checkAccount() { | ||
try { | ||
const cryptoAccount = await getCryptoKeyAccount(); | ||
const newAccount = cryptoAccount?.account; | ||
|
||
// Check if account changed | ||
if (newAccount?.address !== this.account?.address) { | ||
const previousAccount = this.account; | ||
this.account = newAccount; | ||
|
||
// Notify listeners | ||
this.listeners.forEach(listener => { | ||
listener({ | ||
previous: previousAccount, | ||
current: newAccount | ||
}); | ||
}); | ||
} | ||
} catch (error) { | ||
console.error('Error monitoring account:', error); | ||
} | ||
} | ||
|
||
onAccountChange(listener: (account: any) => void) { | ||
this.listeners.push(listener); | ||
} | ||
|
||
getCurrentAccount() { | ||
return this.account; | ||
} | ||
} | ||
``` | ||
|
||
## Best Practices | ||
|
||
1. **Check for null returns**: Always check if account exists before using | ||
2. **Handle errors gracefully**: Wrap calls in try-catch blocks | ||
3. **Cache results**: Avoid excessive calls by caching account information | ||
4. **Monitor changes**: Implement account change detection for better UX | ||
5. **Validate data**: Always verify account data integrity | ||
|
||
import PolicyBanner from "/snippets/PolicyBanner.mdx"; | ||
|
||
<PolicyBanner /> |
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.
let's delete section, this library is meant to be used to manage sub account signers.
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.
deleted, thanks
| Code | Message | Description | | ||
| ---- | ------- | ----------- | | ||
| 4100 | Storage access denied | Cannot access secure key storage | | ||
| 4200 | Storage corruption | Stored key data is corrupted or invalid | | ||
| 4300 | Browser compatibility | Browser does not support required storage APIs | | ||
|
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 AI is hallucinating here, we don't throw these error codes.
} | ||
``` | ||
|
||
## Integration with Account Management |
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.
I would reframe this section as maybe "usage example". I don't want to confuse devs that this is how we want them to do base account account management, since this is not what this library is used for (it's for managing sub account local signers)
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.
I just removed it
```typescript Success Response (WebAuthn Account) | ||
{ | ||
account: { | ||
address: "0xd46e8dd67c5d32be8058bb8eb970870f07244567", |
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.
address: "0xd46e8dd67c5d32be8058bb8eb970870f07244567", |
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.
Left some comments
} | ||
``` | ||
|
||
```json signInWithEthereum Response |
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.
should consider just linking out to the SIWE capability page so that we don't have to maintain this in two places
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.
agree - removed from there for now and added linking to the capability
Defined in the [Base Account SDK](https://github.com/base/account-sdk) | ||
|
||
<Info> | ||
Retrieves the current crypto account associated with the user's session. This is the primary function for accessing account information in Base Account applications. |
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.
finding this hard to understand
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.
updated
Defined in the [Base Account SDK](https://github.com/base/account-sdk) | ||
|
||
<Info> | ||
Retrieves an existing P256 key pair if one has been previously generated and stored. This is useful for checking if keys already exist before generating new ones. |
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.
could we maybe combine this, getCryptoKeyAccount, and generateKeyPair into a single page?
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.
will take a look at this in a next PR
Co-authored-by: Montana <[email protected]>
Approved review 3071896399 from ericbrown99 is now dismissed due to new commit. Re-request for approval.
Co-authored-by: Montana <[email protected]>
Co-authored-by: Montana <[email protected]>
Co-authored-by: Montana <[email protected]>
Co-authored-by: Montana <[email protected]>
What changed? Why?