Skip to content
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

imp(staking): change the validator address in the events from string type to address type #2053

Merged
merged 4 commits into from Nov 21, 2023

Conversation

luchenqun
Copy link
Contributor

@luchenqun luchenqun commented Nov 18, 2023

Description

In #2030, you recommend use address instead of string for validator address in events. I think this is a good idea. Using address instead of string would make it easier for developers to track the corresponding validator, because use string type the recorded value would be keccak256(validatorAddress). Moreover, it would be difficult to decode events and determine which validator the hash corresponds to if string type is used. Therefore, I have updated the string type validator address in other events to address type.

Here is an example of a Delegate event before and after the update

Before Delegate event with validator address as string
keeack256("evmosvaloper1hajh6rhhkjqkwet6wqld3lgx8ur4y3khljfx82")
-->
"0xc30b39324ec8e945bd2d238ee3e5475a8108e9d0c37e1278c4b0ec30283fdacf"

oldevent

Afte Delegate event with validator address as address type
bech32 "evmosvaloper1hajh6rhhkjqkwet6wqld3lgx8ur4y3khljfx82"
<-->
hex "0xbf657D0ef7b48167657A703Ed8Fd063F075246D7"

newevent

@luchenqun luchenqun requested a review from a team as a code owner November 18, 2023 08:33
@luchenqun luchenqun requested review from facs95 and MalteHerrmann and removed request for a team November 18, 2023 08:33
Copy link

codecov bot commented Nov 18, 2023

Codecov Report

Merging #2053 (6b0ad92) into main (e51d4d6) will increase coverage by 0.01%.
Report is 3 commits behind head on main.
The diff coverage is 43.47%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2053      +/-   ##
==========================================
+ Coverage   70.16%   70.18%   +0.01%     
==========================================
  Files         340      340              
  Lines       25448    25443       -5     
==========================================
+ Hits        17856    17857       +1     
+ Misses       6679     6669      -10     
- Partials      913      917       +4     
Files Coverage Δ
precompiles/staking/types.go 74.76% <ø> (ø)
precompiles/staking/events.go 63.30% <43.47%> (+1.86%) ⬆️

Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Looks good to me. There's a minor issue with the formatting

precompiles/staking/events_test.go Outdated Show resolved Hide resolved
@fedekunze fedekunze merged commit 8efa1e3 into evmos:main Nov 21, 2023
19 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants