-
Notifications
You must be signed in to change notification settings - Fork 642
design doc: waddrmgr: add sql schema doc for reference #1051
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
base: sql-wallet
Are you sure you want to change the base?
design doc: waddrmgr: add sql schema doc for reference #1051
Conversation
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.
Very nice work, @GustavoStingelin! The design doc looks pretty good to me. Left some comments. Likely will take another look later after I have some more relevant business knowledge regards btcwallet interactions
3b1c683 to
a47a2c7
Compare
|
added comments to the indexes and also renamed some 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.
Sweet, think we can revisit once the utxo table is out, meanwhile my two cents,
- while modern wallets like
bitciondand BDK use wallet descriptors, we will keep it "traditional" here to make the changes incremental and support customized key scope types where external and internal addresses can use different address types. - we now support creating multiple wallets which is create, tho at the app layer we need to stick to the default wallet for simplicity, otherwise a query to generate new addr can become
wallet name + scope + accountwhich can be cumbersome. - we now support importing at every layer in the bip44 path, master seed, accounts and addresses.
a47a2c7 to
1ab4df8
Compare
|
@yyforyongyu @mohamedawnallah added a lot of changes, pls re-review carefully. |
Missed this notification, @GustavoStingelin. Putting it now on my review queue |
yyforyongyu
left a comment
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.
Overall, I think this is a robust design. There are still details to be addressed, such as whether to use uuidv7, which specific text fields should be checked, and that the _secrets table should only store secrets. I think we can figure these out in the actual implementation and update the docs once we have a deeper understanding of how the schema is used in practice. Thus, I will focus on reviewing the implementation for now.
1ab4df8 to
81072c0
Compare
81072c0 to
b3bc835
Compare
|
The table design is becoming a bit complex. I’m wondering if we have room to simplify it. Maybe by using the descriptor pattern that YY mentioned, we could still cover the required features with a simpler schema. |
|
Concept ACK. I think the current design gives us a very good starting point, and let's revisit this doc once the implementation is done!
Yeah maybe in the far future - switching to descriptor will require too much work, it means we will rewrite the whole thing. |
b3bc835 to
2e4465e
Compare
|
converted to draft since it's just a reference doc |
2e4465e to
a1835d2
Compare
da77be1 to
34fd443
Compare
c3c4f0d to
866b646
Compare
WAddrmgr SQL Schema
This PR introduces SQL schema documentation for the wallet address manager as part of the Workstream 3 SQLization effort #1015, and should be used as a reference for future migration and concrete implementations.
Tables Schema
interactive link
Design Decisions
Why use UUID instead of BIGSERIAL as the primary key?
UUID can be generated at the application layer and stored directly in the database. This removes the need to fetch the ID from the database after insertion and makes parallel and batch operations simpler.
-> changed to INT to achieve simplicity.
Why use BIGINT to store fields like
account_number?These fields are
uint32in the spec, and PostgreSQL does not support unsigned integer types. Using BIGINT ensures no overflow.