mx-chain-go
mx-chain-go copied to clipboard
Add code-hash endpoint to address group
Description of the reasoning behind the pull request (what feature was missing / how the problem was manifesting itself / what was the motive behind the refactoring)
In Arda, we are going to add a service that tracks if smart contract binary codes change.
Currently, it is impossible to only request the code hash. The only way is to download all the account data (including the binary code that can be huge).
Proposed Changes
Add a code-hash endpoint to the address group.
Testing procedure
@bogdan-rosianu @iulianpascalau I have forced pushed to add changes you requested and get the commit signed.
I think I fixed all the issues you reported except one about the following code in node.go
:
}
codeHash := userAccount.GetCodeHash()
return string(codeHash), blockInfo, nil
-
I don't understand what is wrong with this. I checked the code in
nodeFacade.GetAccount()
but don't understand whyGetCode
should appear inGetCodeHash
. -
You told me to return an hex version of the code hash. I did the modification. However, it seems I should rather return a base64 version of the code hash, isn't it? Because in the endpoint /address/:address, the code hash is base64 encoded.
@bogdan-rosianu @iulianpascalau I have forced pushed to add changes you requested and get the commit signed.
I think I fixed all the issues you reported except one about the following code in
node.go
:} codeHash := userAccount.GetCodeHash() return string(codeHash), blockInfo, nil
- I don't understand what is wrong with this. I checked the code in
nodeFacade.GetAccount()
but don't understand whyGetCode
should appear inGetCodeHash
.- You told me to return an hex version of the code hash. I did the modification. However, it seems I should rather return a base64 version of the code hash, isn't it? Because in the endpoint /address/:address, the code hash is base64 encoded.
Let's see: the userAccount.GetCodeHash() returns the plain []byte slice (usually will be here 32 bytes). These will almost surely contain unreadable characters. Like /0
or /1
and so forth. If you just cast it to string, the webserver will think that this is a valid string and, I think, will not try to encode it even further. The suggestion to return the hex encoded code hash was one suggestion, but of course it can be encoded into a base64 form.
To comply with the "clean code" guidance, indeed, you should do the encoding only on the most outer layer of the plugin involved and pass the code hash as []byte slice everywhere else (node.go, the facade, interfaces involved and so on).
So, in addressGroup.go L245 you should put:
shared.RespondWithSuccess(c, gin.H{"codeHash": base64.StdEncoding.EncodeToString(codeHash), "blockInfo": blockInfo})
Or you can use hex. We mostly return hex encoded stuff as it is easier to handle on some frontends.
Thanks a lot @iulianpascalau for the time you took to explain me! I added a new commit that should address the remaining issue
Thanks a lot @bogdan-rosianu for your review and time too!
@iulianpascalau Two tests were failing. Sorry for that! I fixed them now.
@bogdan-rosianu Thanks for the review. I removed the useless encoding
Codecov Report
:exclamation: No coverage uploaded for pull request base (
external-codehash-endpoint@65ccced
). Click here to learn what that means. The diff coverage isn/a
.
:exclamation: Current head 3f6de7b differs from pull request most recent head 99dd450. Consider uploading reports for the commit 99dd450 to get more accurate results
@@ Coverage Diff @@
## external-codehash-endpoint #4391 +/- ##
=============================================================
Coverage ? 73.67%
=============================================================
Files ? 678
Lines ? 86911
Branches ? 0
=============================================================
Hits ? 64032
Misses ? 18026
Partials ? 4853
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Thank you!!
Hi @iulianpascalau @bogdan-rosianu Has this PR merged in the codebase somewhere? The branch external-codehash-endpoint doesn't exist anymore. And the code of the PR doesn't seem to be in development or master.
By the way, the product that would benefit from this endpoint has been launched. It is Arda Audits: https://arda.run/audits
Hello. We've planned to phase out the development
branch. The docs will be updated soon to specify where the external PRs should be started from.
However, before phasing out the development
branch, we've merged everything to rc/v1.4.0
since the release we've just rolled out (v1.3.42) was feature-frozen.
TL;DR, the changes can be found on rc/v1.4.0
-> the upcoming planned release
https://github.com/ElrondNetwork/elrond-go/blob/894a3d7c29c8b4f24030679ca46f5339466cadce/api/groups/addressGroup.go#L23
Great @iulianpascalau ! Thank you for the precisions!