Skip to content
This repository was archived by the owner on Oct 6, 2025. It is now read-only.

Add Eth1 address withdrawal support#195

Merged
hwwhww merged 7 commits intodevfrom
eth1-address-withdrawal
Mar 30, 2021
Merged

Add Eth1 address withdrawal support#195
hwwhww merged 7 commits intodevfrom
eth1-address-withdrawal

Conversation

@hwwhww
Copy link
Copy Markdown
Contributor

@hwwhww hwwhww commented Mar 25, 2021

Fix #188, support ethereum/consensus-specs#2149

  • cli interface: add --eth1_withdrawal_address argument: If this field is set and valid, the given Eth1 address will be used to create the withdrawal credentials. Otherwise, it will generate withdrawal credentials with the mnemonic-derived withdrawal public key.
  • validate_deposit: add public key and withdrawal credentials validations.

@hwwhww hwwhww requested a review from CarlBeek March 25, 2021 15:26
def validate_eth1_withdrawal_address(cts: click.Context, param: Any, address: str) -> HexAddress:
if address is None:
return None
if not is_hex_address(address):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: the prefix 0x is optional here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hello word

@hwwhww
Copy link
Copy Markdown
Contributor Author

hwwhww commented Mar 25, 2021

/cc @djrtwo
Considering your de-Eth1/Eth2 rebranding, any plan to rename ETH1_ADDRESS_WITHDRAWAL_PREFIX?

Copy link
Copy Markdown
Collaborator

@CarlBeek CarlBeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that this is being added. 👍

One issue is that by adding this functionality to generate_keys it is automagically added to both the new-mnemonic and existing-mnemonic commands. The latter of which doesn't have tests and isn't mentioned in the README.

Comment thread eth2deposit/credentials.py
Comment thread eth2deposit/utils/validation.py
@hwwhww hwwhww changed the base branch from master to dev March 30, 2021 11:16
Copy link
Copy Markdown
Collaborator

@CarlBeek CarlBeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Let's get it merged.

@CarlBeek CarlBeek added the enhancement New feature or request label Mar 30, 2021
@hwwhww hwwhww merged commit 27abb5c into dev Mar 30, 2021
@hwwhww hwwhww deleted the eth1-address-withdrawal branch March 30, 2021 12:23
@CarlBeek CarlBeek mentioned this pull request Apr 2, 2021
1 task
Copy link
Copy Markdown

@G-11-P G-11-P left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit and merge

sangheraio pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support ETH1_ADDRESS_WITHDRAWAL_PREFIX withdrawal credentials (0x01)

4 participants