Fee recipient: checksum log #10664
Fee recipient: checksum log #10664
Conversation
james-prysm
added
Merge
beacon-chain/node/config.go
Outdated
| } | ||
| checksumAddress := common.HexToAddress(ha) | ||
| if !mixedcaseAddress.ValidChecksum() { | ||
| log.Warnf("fee recipient %s is not a valid checksum address. The checksummed address is %s and this will be used as the fee recipient", ha, checksumAddress.Hex()) |
There was a problem hiding this comment.
Let's consider this log from the user's perspective. Many do not know what a checksum is, so we should instead warn them that a checksummed address is safer to prevent against spelling mistakes in Ethereum addresses. The provided address was not checksummed and will be used as is when setting transaction fee recipients
There was a problem hiding this comment.
i'll go ahead and update this
There was a problem hiding this comment.
Maybe something about how we recommend using a mixed-case address to prevent against spelling mistakes in your fee recipient Ethereum address. The provided address is not a valid checksum (mixed-case) address. Not sure if we should link people to a specific place, but something worth thinking about
What type of PR is this?
Feature
What does this PR do? Why is it needed?
Reopening PR for checksum check for fee recipient.
Users should be aware when they use a non checksummed fee recipient. This new validation will warn users that they are using the non checksummed version but using the correct address with all lower case(invalid checksum) would still go to the same destination ( correct checksum address)
Which issues(s) does this PR fix?
Fixes #10631