[Flow EVM] return empty string when deployed address is empty #5456
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5456 +/- ##
==========================================
+ Coverage 56.02% 57.47% +1.44%
==========================================
Files 1026 848 -178
Lines 99940 84400 -15540
==========================================
- Hits 55990 48507 -7483
+ Misses 39661 32112 -7549
+ Partials 4289 3781 -508
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
fvm/evm/types/address.go
Outdated
| if fa == EmptyAddress { | ||
| return "" | ||
| } | ||
| return hex.EncodeToString(fa[:]) |
There was a problem hiding this comment.
Should we maybe change this to:
return fa.ToCommon().Hex()?
The difference being that the latter returns the check-summed variant, e.g.:
from: 99466ed2e37b892a2ee3e9cd55a98b68f5735db2
to: 0x99466ED2E37B892A2Ee3E9CD55a98b68f5735db2
There was a problem hiding this comment.
Note: I also made a change in #5416, which I guess is conflicting.
There was a problem hiding this comment.
good idea, I'll use your changes, but I think the issue is still the empty address, it would result in growth in all events.
There was a problem hiding this comment.
now that I'm looking into it, the checksum is a post-process only input-dependent, not sure if it makes sense to do it on-chain and not off-chain later?
There was a problem hiding this comment.
Yeah, the empty address should be omitted and simply return a ""
now that I'm looking into it, the checksum is a post-process only input-dependent, not sure if it makes sense to do it on-chain and not off-chain later?
That's true, I only mentioned to have it check-summed in the event payload, to showcase its validity. It is something that can be done off-chain as well though. It does not make a difference.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
ramtinms
deleted the
ramtin/flow-evm-dont-returne-address-zero-for-deployment
branch
No description provided.