imp(staking): replace bech32 address with evm hex address for staking precompile contract #2200
Conversation
…e and undelegate function
luchenqun
requested review from
Vvaradinov and
GAtom22
and removed request for
a team
|
Looks good @luchenqun, thanks for your contribution! I do agree it is a good idea to work with the evm hex in the precompile. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Hello Tom, because I have changed the |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2200 +/- ##
==========================================
- Coverage 70.53% 70.48% -0.06%
==========================================
Files 291 291
Lines 22617 22639 +22
==========================================
+ Hits 15953 15956 +3
- Misses 5798 5811 +13
- Partials 866 872 +6
|
I modified it based on my intuition, and it seems to be OK. LOL |
|
@fedekunze @Vvaradinov It took an hour to implement the feature, but a day to fix the unit tests. Hahaha. Now you can start review the updated code. |
| @@ -334,15 +332,12 @@ contract StakingCaller { | |||
| (shares, coin) = abi.decode(data, (uint256, staking.Coin)); | |||
| } else if (calltypeHash == keccak256(abi.encodePacked("callcode"))) { | |||
| //Function signature | |||
| bytes4 sig = bytes4(keccak256(bytes("delegation(address,string)"))); | |||
| bytes4 sig = bytes4(keccak256(bytes("delegation(address,address)"))); | |||
There was a problem hiding this comment.
We need @GAtom22 to verify this change, I don't enough of solidity to be sure that it's the correct change (it looks good)
| }, | ||
| { | ||
| "success - undelegate transaction with correct gas estimation", | ||
| func() []byte { | ||
| input, err := s.precompile.Pack( | ||
| staking.UndelegateMethod, | ||
| s.address, | ||
| s.validators[0].GetOperator().String(), | ||
| common.BytesToAddress(s.validators[0].GetOperator().Bytes()), |
There was a problem hiding this comment.
We need to a typescript library to convert from validator bech32 to hex in order to update the frontend. Without the lib the frontend will not be able to create staking transactions after upgrading the network with the new code.
|
Providing a Typescript lib to convert the validators addresses is a blocker to merge this PR. After having the lib ready we can finish the PR review and merge it. @luchenqun I'll move the pr to draf until we have a tested typescript lib that works fine for this change, I guess that in theory the code provided by the now deprecated Evmosjs may work, but I have never tried it for validator addresses. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs. |
|
Not Stale |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs. |
Description
This PR will convert all bech32 addresses in precompiled contract staking to evm hex address. The implementation itself is not difficult, but too many unit tests need to be fixed. I originally wanted to fix one function such as delegate with one PR, but I found that there is a strong correlation between all functions, and it would be more efficient to fix them all at once. It is roughly planned to complete all tasks by the end of January next year.
Please help me review whether the changes to the StakingI.sol file are reasonable, because subsequent updates need to be based on the StakingI.sol file. @fedekunze @Vvaradinov
More details see before #2122