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.

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

fix: Set the designated payer when building an EVM transactions from scheduled transactions #16708

Merged
merged 96 commits into from
Nov 28, 2024

Conversation

JivkoKelchev
Copy link
Contributor

@JivkoKelchev JivkoKelchev commented Nov 21, 2024

Description:
Updates the hevmTransactionFactory to accept the payer instead of extracting it from the transaction ID, while building from hapi transaction. This is needed because scheduled transactions inherited their ID from scheduleCreate transactions, and may have a different payer.

Related issue(s):

Fixes #

Notes for reviewer:

Checklist

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

JivkoKelchev and others added 30 commits October 16, 2024 10:13
Signed-off-by: Valentin Tronkov <[email protected]>
Signed-off-by: ibankov <[email protected]>
…transactions

# Conflicts:
#	hedera-node/hedera-app/src/main/java/com/hedera/node/app/workflows/handle/HandleWorkflow.java
Signed-off-by: Zhivko Kelchev <[email protected]>
refactor hapi tests

Signed-off-by: Zhivko Kelchev <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Zhivko Kelchev <[email protected]>
Signed-off-by: Zhivko Kelchev <[email protected]>
Signed-off-by: Zhivko Kelchev <[email protected]>
fix long term schedule throttle warning.

Signed-off-by: Zhivko Kelchev <[email protected]>
…persistence' into 15055-POC-long-term-scheduled-transactions

# Conflicts:
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/HandlerUtility.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleCreateHandler.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleDeleteHandler.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleManager.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleSignHandler.java
#	hedera-node/hedera-schedule-service-impl/src/test/java/com/hedera/node/app/service/schedule/impl/handlers/HandlerUtilityTest.java
#	hedera-node/hedera-schedule-service-impl/src/test/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleCreateHandlerTest.java
#	hedera-node/hedera-schedule-service-impl/src/test/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleManagerTest.java
Signed-off-by: Zhivko Kelchev <[email protected]>
…transactions

# Conflicts:
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/AbstractScheduleHandler.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/HandlerUtility.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleCreateHandler.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleDeleteHandler.java
#	hedera-node/hedera-schedule-service-impl/src/main/java/com/hedera/node/app/service/schedule/impl/handlers/ScheduleSignHandler.java
…transactions

# Conflicts:
#	hedera-node/hedera-app/src/test/java/com/hedera/node/app/workflows/handle/HandleWorkflowTest.java
#	hedera-node/test-clients/src/main/java/com/hedera/services/bdd/spec/queries/schedule/HapiGetScheduleInfo.java
Signed-off-by: Zhivko Kelchev <[email protected]>
Signed-off-by: Zhivko Kelchev <[email protected]>
Copy link

codecov bot commented Nov 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.53%. Comparing base (0c23fbb) to head (3009d36).
Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             develop   #16708      +/-   ##
=============================================
- Coverage      63.53%   63.53%   -0.01%     
+ Complexity     20372    20371       -1     
=============================================
  Files           2537     2537              
  Lines          94757    94754       -3     
  Branches        9904     9904              
=============================================
- Hits           60206    60200       -6     
- Misses         30946    30948       +2     
- Partials        3605     3606       +1     
Files with missing lines Coverage Δ
...ontract/impl/exec/ContextTransactionProcessor.java 80.55% <100.00%> (ø)
...ce/contract/impl/infra/HevmTransactionFactory.java 92.30% <100.00%> (-0.18%) ⬇️

... and 1 file with indirect coverage changes

Impacted file tree graph

Copy link

codacy-production bot commented Nov 21, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.00% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (0c23fbb) 97522 63622 65.24%
Head commit (3009d36) 97519 (-3) 63617 (-5) 65.24% (+0.00%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#16708) 4 4 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@Nana-EC
Copy link
Contributor

Nana-EC commented Nov 25, 2024

@JivkoKelchev can you add more details to this PR.
Also let's discuss this a bit more, my understanding is this is enabling the scheduling of a ContractCall transaction.
Can we enable this only in dev & previewnet at this juncture while this is explored more.
Same for all Smart contract transactions
FYI @netopyr and @lukelee-sl

tinker-michaelj and others added 2 commits November 25, 2024 01:28
Signed-off-by: Michael Tinker <[email protected]>
Signed-off-by: Zhivko Kelchev <[email protected]>
Co-authored-by: Zhivko Kelchev <[email protected]>
Co-authored-by: Joseph S. <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
@JivkoKelchev
Copy link
Contributor Author

Hi @Nana-EC ,

This change was needed because scheduled transactions inherit their transaction ID from their initial ScheduleCreate transaction.

In this case, we want a custom payer that will pay the gas. So we need a way to provide the payer (contract call sender) instead of extracting the one from the transaction ID, while creating the HederaEvmTransaction here.

However, I’m not really sure if we still need these changes, since @tinker-michaelj did significant refactoring of the long-term schedule implementation. What do you think Michael?

…into 15055-long-term-schedule-contract-call

# Conflicts:
#	hedera-node/hedera-smart-contract-service-impl/src/test/java/com/hedera/node/app/service/contract/impl/test/exec/ContextTransactionProcessorTest.java
Signed-off-by: Zhivko Kelchev <[email protected]>
Base automatically changed from 15055-POC-long-term-scheduled-transactions to develop November 25, 2024 15:03
@JivkoKelchev JivkoKelchev marked this pull request as ready for review November 27, 2024 15:27
@JivkoKelchev JivkoKelchev changed the title feat: Long term schedule contract call fix: Set the designated payer when building an EVM transactions from scheduled transactions Nov 27, 2024
Copy link
Collaborator

@tinker-michaelj tinker-michaelj left a comment

Choose a reason for hiding this comment

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

LGTM, tyvm @JivkoKelchev !

@tinker-michaelj tinker-michaelj merged commit 0759d3d into develop Nov 28, 2024
64 of 65 checks passed
@tinker-michaelj tinker-michaelj deleted the 15055-long-term-schedule-contract-call branch November 28, 2024 02:50
JivkoKelchev added a commit that referenced this pull request Nov 28, 2024
…scheduled transactions (#16708)

Signed-off-by: Zhivko Kelchev <[email protected]>
Signed-off-by: Valentin Tronkov <[email protected]>
Signed-off-by: ibankov <[email protected]>
Signed-off-by: Michael Tinker <[email protected]>
Co-authored-by: Valentin Tronkov <[email protected]>
Co-authored-by: ibankov <[email protected]>
Co-authored-by: Michael Tinker <[email protected]>
Co-authored-by: Joseph S. <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants