-
Notifications
You must be signed in to change notification settings - Fork 702
Add ERC: HD wallet In Treasury Management #978
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
Add ERC: HD wallet In Treasury Management #978
Conversation
✅ All reviewers have approved. |
Discuss To:https://ethereum-magicians.org/t/new-erc-deterministic-account-hierarchy-in-treasury-management/23073 |
ERCS/erc-treasury_hd_wallet.md
Outdated
```python | ||
entity_hash = sha256(f"ENTITY:{entity}".encode()).digest() | ||
entity_index = (int.from_bytes(entity_hash[:4], "big") % 2**31) + 2**31 # 2^31 ≤ index < 2^32 | ||
``` | ||
|
||
2. **Department Index Calculation** | ||
|
||
- **MUST** compute `department_id` as: | ||
|
||
```python | ||
dept_hash = sha256(f"DEPT:{entity_hash}:{department}".encode()).digest() | ||
dept_index = (int.from_bytes(dept_hash[:4], "big") % 2^31) + 2^31 | ||
``` | ||
|
||
3. **Output Constraints** | ||
|
||
- Generated indices **MUST** be integers in `[2^31, 2^32-1]` to enforce hardened derivation. |
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 current calculation method in this proposal is (89-90):
entity_hash = sha256(f"ENTITY:{entity}".encode()).digest()
entity_index = (int.from_bytes(entity_hash[:4], "big") % 2**31) + 2**31
Some problems to this:
Range Limitation: looking at line 104 it states indices must be in the range [2^31, 2^32-1]
for hardened derivation, the modulo operation unnecessarily restricts the possible values, reducing entropy.
Entropy Loss: Taking the modulo of the hash value with 2^31
means we're only using approximately 30 bits of entropy from the original 32 bits (4 bytes), potentially increasing collision risk.
Implementation Inconsistency: The code example uses % 2^31
which in Python would be interpreted as a bitwise XOR operation rather than exponentiation (should be % 2**31
)?
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.
Thank you for your comments, I have optimized logic in the last pr: remain 32 bits to decrease collision risk and modify typo
ERCS/erc-treasury_hd_wallet.md
Outdated
|
||
6. **Account Index (**`account_index`**)** | ||
|
||
- **MUST** use non-hardened derivation to allow unified 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.
What happens if the parent public key is compromised?
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 design of unreinforced derivation makes it possible for the parent private key to be derived once both of the following conditions are met:
- The parent public key (xpub) and chain code are known (xpub is public).
- A child private key is leaked.
but convenience and risk need to balance~ and risk notify has been added in chapter "Security Considerations"
ERCS/erc-treasury_hd_wallet.md
Outdated
This proposal aims to provide a standardized method for on-chain treasury management of institutional assets, ensuring | ||
secure private key generation, hierarchical management, and departmental permission isolation while supporting asset | ||
security and transaction efficiency in multi-chain environments. By defining a unified derivation path and security | ||
mechanisms, this proposal offers an efficient and secure solution for treasury 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.
This proposal aims to provide a standardized method for on-chain treasury management of institutional assets, ensuring | |
secure private key generation, hierarchical management, and departmental permission isolation while supporting asset | |
security and transaction efficiency in multi-chain environments. By defining a unified derivation path and security | |
mechanisms, this proposal offers an efficient and secure solution for treasury management. | |
This proposal aims to provide a standardized method for on-chain treasury management of institutional assets, ensuring secure private key generation, hierarchical management, and departmental permission isolation while supporting asset security and transaction efficiency in multi-chain environments. By defining a unified derivation path and security mechanisms, this proposal offers an efficient and secure solution for treasury 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.
updated
ERCS/erc-treasury_hd_wallet.md
Outdated
With the rapid development of blockchain and DeFi, secure management of on-chain assets has become critical. Traditional | ||
private key management struggles to meet the security demands of large organizations in complex scenarios, where | ||
hierarchical key management, permission controls, and multi-signature mechanisms are essential. This proposal provides a | ||
standardized solution for institutional treasury management, ensuring asset security and transaction efficiency. |
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.
With the rapid development of blockchain and DeFi, secure management of on-chain assets has become critical. Traditional | |
private key management struggles to meet the security demands of large organizations in complex scenarios, where | |
hierarchical key management, permission controls, and multi-signature mechanisms are essential. This proposal provides a | |
standardized solution for institutional treasury management, ensuring asset security and transaction efficiency. | |
With the rapid development of blockchain and DeFi, secure management of on-chain assets has become critical. Traditional private key management struggles to meet the security demands of large organizations in complex scenarios, where hierarchical key management, permission controls, and multi-signature mechanisms are essential. This proposal provides a standardized solution for institutional treasury management, ensuring asset security and transaction efficiency. |
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
ERCS/erc-treasury_hd_wallet.md
Outdated
For treasury managers, hierarchical deterministic wallet management is more convenient, but it requires additional | ||
consideration of protective measures for the master key, such as schemes for splitting and storing mnemonic phrases or | ||
master keys. |
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.
For treasury managers, hierarchical deterministic wallet management is more convenient, but it requires additional | |
consideration of protective measures for the master key, such as schemes for splitting and storing mnemonic phrases or | |
master keys. | |
For treasury managers, hierarchical deterministic wallet management is more convenient, but it requires additional consideration of protective measures for the master key, such as schemes for splitting and storing mnemonic phrases or master keys. |
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
…o treasury_hd_wallet_proposal
@eip-review-bot can scan again? |
The commit cf60b35 (as a parent of 7f90365) contains errors. |
@eip-review-bot @g11tech @SamWilsn @xinbenlv , could you kindly review this pull request? All checks have been successfully completed. Thank you! |
Friendly reminder to keep technical discussion on the discussion thread and not on the pull requests. Discussions here should only be for formatting and editorial comments. It's too easy to lose context across pull requests otherwise. |
Hi,SamWilsn @SamWilsn , can kindly help to pass merge request? I have already passed all auto checks. Thank you very much~ |
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.
This is fine for a draft, though you should address these comments before moving to review.
As a general note, we prefer the form "BIP XX" (no dash or leading zeroes).
--- | ||
eip: 7908 | ||
title: HD wallet In Treasury Management | ||
description: HD wallets for treasury systems, isolating entities and departments via cryptographic key paths. |
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.
HD wallets for treasury systems,
This section of your description is basically just restating your title, which is an inefficient use of the limited space in the description field. I'd remove it.
Instead, you might consider expanding "HD" or perhaps describing a use case.
|
||
- **SHALL** represent the root HD wallet private key. | ||
|
||
2. **BIP-44 Compliance Layer (`44'`)** |
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.
2. **BIP-44 Compliance Layer (`44'`)** | |
2. **[BIP 44](https://github.com/bitcoin/bips/blob/0278bf3d7111bab8f0ef8dd08f16fd7b5ac6cbd6/bip-0044.mediawiki) Compliance Layer (`44'`)** |
|
||
**Cryptographic Requirements**: | ||
|
||
- **SHALL** use BIP-0032 with `secp256k1` for HD derivation. |
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.
- **SHALL** use BIP-0032 with `secp256k1` for HD derivation. | |
- **SHALL** use [BIP 32](https://github.com/bitcoin/bips/blob/a3a397c82384220fc871852c809f73898a4d547c/bip-0032.mediawiki) with `secp256k1` for HD derivation. |
|
||
## Backwards Compatibility | ||
|
||
This standard complies with BIP39、BIP32、BIP44. |
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.
This standard complies with BIP39、BIP32、BIP44. | |
This standard complies with [BIP 39](https://github.com/bitcoin/bips/blob/b9f9a8d6e854fa0b0c8f818753420b0aa6e875aa/bip-0039.mediawiki), [BIP 32](https://github.com/bitcoin/bips/blob/a3a397c82384220fc871852c809f73898a4d547c/bip-0032.mediawiki), [BIP 44](https://github.com/bitcoin/bips/blob/0278bf3d7111bab8f0ef8dd08f16fd7b5ac6cbd6/bip-0044.mediawiki). |
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.
All Reviewers Have Approved; Performing Automatic Merge...
When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md
We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met: