Skip to content

Conversation

kselveliev
Copy link
Contributor

@kselveliev kselveliev commented Feb 10, 2025

Description:

This PR fixes ContractCallNestedCallsHistoricalTest tests against modularized code.

Related issue(s):

Fixes #10081

Notes for reviewer:

Checklist

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

Signed-off-by: Kristiyan Selveliev <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
@kselveliev kselveliev added this to the 0.124.0 milestone Feb 10, 2025
@kselveliev kselveliev self-assigned this Feb 10, 2025
@kselveliev kselveliev requested a review from a team as a code owner February 10, 2025 11:21
Copy link

codecov bot commented Feb 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.21%. Comparing base (442be2b) to head (813b433).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main   #10349   +/-   ##
=========================================
  Coverage     92.21%   92.21%           
  Complexity     8025     8025           
=========================================
  Files           982      982           
  Lines         33507    33507           
  Branches       4226     4226           
=========================================
  Hits          30900    30900           
  Misses         1607     1607           
  Partials       1000     1000           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

final var tokenMemo = "TestMemo";
final var ownerEntity = accountEntityNoEvmAddressPersistHistorical(
Range.closedOpen(recordFileBeforeEvm34.getConsensusStart(), recordFileBeforeEvm34.getConsensusEnd()));
final var spenderPublicKey = "3a210398e17bcbd2926c4d8a31e32616b4754ac0a2fc71d7fb768e657db46202625f34";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we use the key that the domain builder would generate, if we set no value at all? Or at least the SPENDER* constants from ContractCallTestUtil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored to use the spender constants.

Copy link
Contributor Author

@kselveliev kselveliev Feb 10, 2025

Choose a reason for hiding this comment

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

domainBuilder.key() does not work. We have a new method which it calls -> generateSecp256k1Key which is used to generated compressed public key. But it not always generates a valid key and the test becomes flaky (recoverAddressFromPubKey starts failing). This need to be fixed in another pr.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please open an issue to fix the domainBuilder.key() method then?

Signed-off-by: Kristiyan Selveliev <[email protected]>
Signed-off-by: Kristiyan Selveliev <[email protected]>
Copy link

@steven-sheehy steven-sheehy added the enhancement Type: New feature label Feb 10, 2025
Copy link
Contributor

@bilyana-gospodinova bilyana-gospodinova left a comment

Choose a reason for hiding this comment

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

LGTM

@steven-sheehy steven-sheehy merged commit 7115756 into main Feb 10, 2025
35 of 36 checks passed
@steven-sheehy steven-sheehy deleted the 10081-nested-calls-historical branch February 10, 2025 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type: New feature modularized evm web3 Area: Web3 API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix ContractCallNestedCallsHistoricalTest test suite
3 participants