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.

Sign up for GitHub

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

Jump to bottom

Populate EVM Address in CryptoTranscation #5010

Merged
merged 15 commits into from Mar 16, 2023
Merged

Populate EVM Address in CryptoTranscation #5010

merged 15 commits into from Mar 16, 2023

Conversation

randered
Copy link
Collaborator

@randered randered commented Feb 17, 2023 •
edited

Description:

Populates the EVM address, when account is created via CryptoTransfer to EDCSA key.
Related issue(s):

Fixes #5009

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@randered
added test and logic for #5009
481ccfb 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
@randered randered self-assigned this Feb 17, 2023
@github-actions
Copy link

github-actions bot commented Feb 17, 2023 •
edited

Unit Test Results

  1 222 files    1 222 suites   33m 52s ⏱️
96 650 tests 96 649 ✔️ 1 💤 0 ❌
98 276 runs  98 275 ✔️ 1 💤 0 ❌

Results for commit ddeba21.

♻️ This comment has been updated with latest results.

@randered randered requested review from dimitar-dinev and MiroslavGatsanoga and removed request for dimitar-dinev February 17, 2023 12:42
@github-actions
Copy link

github-actions bot commented Feb 17, 2023 •
edited

Integration Test Results

106 tests   106 ✔️  10m 14s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit ddeba21.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Feb 17, 2023 •
edited

E2E Test Results

    1 files      1 suites   17m 25s ⏱️
308 tests 308 ✔️ 0 💤 0 ❌
326 runs  326 ✔️ 0 💤 0 ❌

Results for commit ddeba21.

♻️ This comment has been updated with latest results.

@randered
fixed logic and naming
2717928 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
@randered
fixed SonarCloud
66bc519 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
@randered randered added the Limechain Work planned for the LimeChain team label Feb 20, 2023
@randered
fixed typo
a74358b 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
@randered
fixed typo and merged with develop
f7d6a57 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
@randered
uncommented tests
3adcd8f 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
stoqnkpL
stoqnkpL previously approved these changes Feb 20, 2023
View reviewed changes
Copy link
Collaborator

@stoqnkpL stoqnkpL left a comment

Choose a reason for hiding this comment

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

LGTM! Just needs some coverage for the new lines.

@randered
removed unnecessary check
88cd1b5 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
stoqnkpL
stoqnkpL previously approved these changes Feb 21, 2023
View reviewed changes
@randered randered marked this pull request as ready for review February 21, 2023 11:40
dimitar-dinev
dimitar-dinev reviewed Feb 21, 2023
View reviewed changes
Copy link
Collaborator

@dimitar-dinev dimitar-dinev left a comment •
edited

Choose a reason for hiding this comment

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

IMO we need unit tests verifying the new behavior of setting the evm address in the child record.

Apart from that, LG

@randered
added assertion for EVM address
97040de 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
@randered
added assertion for creation with ED key
e64e4c5 
Signed-off-by: Vasil Vasilev <vasil.yanev15@gmail.com>
mustafauzunn
mustafauzunn previously approved these changes Feb 27, 2023
View reviewed changes
Copy link
Collaborator

@mustafauzunn mustafauzunn left a comment

Choose a reason for hiding this comment

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

LGTM

Neeharika-Sompalli
Neeharika-Sompalli previously approved these changes Mar 9, 2023
View reviewed changes
Copy link
Member

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

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

LGTM !

@randered randered requested a review from a team as a code owner March 13, 2023 08:54
@github-actions
Copy link

github-actions bot commented Mar 13, 2023 •
edited

Node: Unit Test Results

  1 286 files    1 286 suites   54m 11s ⏱️
97 031 tests 97 029 ✔️ 2 💤 0 ❌
98 668 runs  98 666 ✔️ 2 💤 0 ❌

Results for commit 0c0257c.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 13, 2023 •
edited

Node: Integration Test Results

108 tests   108 ✔️  10m 24s ⏱️
    2 suites      0 💤
    2 files        0 ❌

Results for commit 0c0257c.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Mar 13, 2023 •
edited

Node: E2E Test Results

    1 files      1 suites   17m 25s ⏱️
308 tests 308 ✔️ 0 💤 0 ❌
326 runs  326 ✔️ 0 💤 0 ❌

Results for commit 0c0257c.

♻️ This comment has been updated with latest results.

@sonarcloud
Copy link

sonarcloud bot commented Mar 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@randered randered merged commit acc66ba into develop Mar 16, 2023
8 of 9 checks passed
@randered randered deleted the 05009-populate-evm-address branch March 16, 2023 09:09
@nickpoorman nickpoorman added this to the v0.37.0 milestone Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiroslavGatsanoga MiroslavGatsanoga MiroslavGatsanoga approved these changes

@Neeharika-Sompalli Neeharika-Sompalli Neeharika-Sompalli approved these changes

@dimitar-dinev dimitar-dinev Awaiting requested review from dimitar-dinev

@tinker-michaelj tinker-michaelj Awaiting requested review from tinker-michaelj

@mustafauzunn mustafauzunn Awaiting requested review from mustafauzunn

@stoqnkpL stoqnkpL Awaiting requested review from stoqnkpL

Assignees

@randered randered

Labels
Limechain Work planned for the LimeChain team
Projects
None yet
Milestone
v0.37
Development

Successfully merging this pull request may close these issues.

[HIP-583] Populate the evm_address field in the transaction record when it's missing from the transaction body
8 participants
@randered @MiroslavGatsanoga @dimitar-dinev @tinker-michaelj @Neeharika-Sompalli @stoqnkpL @mustafauzunn @nickpoorman