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

feat: Creates design doc for enabling access to hts token evm address #3258

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

konstantinabl
Copy link
Collaborator

@konstantinabl konstantinabl commented Nov 13, 2024

Description:

Creates a design doc for new change to trasnaction receipt format, related to hts token address
Reference: hashgraph/hedera-smart-contracts#901

Related issue(s):

Fixes #3257

@konstantinabl konstantinabl requested a review from a team as a code owner November 13, 2024 18:14
Copy link

github-actions bot commented Nov 13, 2024

Test Results

 19 files  + 18  254 suites  +247   30m 38s ⏱️ + 30m 8s
609 tests +604  604 ✅ +600  4 💤 +4  1 ❌ ±0 
699 runs  +663  692 ✅ +657  6 💤 +6  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 06ba64b. ± Comparison against base commit ce43b76.

This pull request removes 1 and adds 605 tests. Note that renamed tests count towards both.
"before all" hook in "@api-batch-2 RPC Server Acceptance Tests" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-2 RPC Server Acceptance Tests "before all" hook in "@api-batch-2 RPC Server Acceptance Tests"
"before all" hook in "Debug API Test Suite" ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests Debug API Test Suite "before all" hook in "Debug API Test Suite"
"eth_call" for non-existing contract address returns 0x ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call "eth_call" for non-existing contract address returns 0x
001 Should call pureMultiply ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With evm address 001 Should call pureMultiply
001 Should call pureMultiply ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With long-zero address 001 Should call pureMultiply
002 Should call msgSender ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With evm address 002 Should call msgSender
002 Should call msgSender ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With long-zero address 002 Should call msgSender
003 Should call txOrigin ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With evm address 003 Should call txOrigin
003 Should call txOrigin ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With long-zero address 003 Should call txOrigin
004 Should call msgSig ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With evm address 004 Should call msgSig
004 Should call msgSig ‑ RPC Server Acceptance Tests Acceptance tests @api-batch-3 RPC Server Acceptance Tests eth_call Caller contract With long-zero address 004 Should call msgSig
…

♻️ This comment has been updated with latest results.

@konstantinabl konstantinabl added the design Design, pilot and prototyping exploration work label Nov 13, 2024
@konstantinabl konstantinabl added this to the 0.61.0 milestone Nov 13, 2024
Signed-off-by: Konstantina Blazhukova <[email protected]>
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Nice start,
More details need to be highlighted in the design doc to give a clear idea of approach.
See docs under https://github.com/hashgraph/hedera-json-rpc-relay/tree/main/docs/design for inspiration

docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved

N.B Currently, HTS supports both v1 and v2 security model function selectors

1. Extract the token address from the call_result field in the transaction response.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What approach are you intending to utilize to extract?
Is it just brute force string manipulation or are you using additional logic to parse the bytes into an expected format and pull the address from that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Brute force manipulation I would say, Improved the description

Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
Signed-off-by: Konstantina Blazhukova <[email protected]>
@konstantinabl konstantinabl changed the title feat: Creates design doc for enabling access to newly created hts token add… feat: Creates design doc for enabling access to hts token evm address Nov 14, 2024
@konstantinabl konstantinabl added the Internal For changes that affect the project's internal workings but not its outward-facing functionality. label Nov 14, 2024
Copy link
Collaborator

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

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

Nice improvements.
Signing off so as to not block but please adopt some form of my suggestions

docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Outdated Show resolved Hide resolved
docs/design/hts_address_tx_receipt.md Show resolved Hide resolved
Nana-EC
Nana-EC previously approved these changes Nov 14, 2024
Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: konstantinabl <[email protected]>
Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: konstantinabl <[email protected]>
Copy link

sonarcloud bot commented Nov 15, 2024

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.88%. Comparing base (ce43b76) to head (06ba64b).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3258      +/-   ##
==========================================
+ Coverage   77.84%   77.88%   +0.03%     
==========================================
  Files          66       66              
  Lines        4460     4468       +8     
  Branches     1000     1003       +3     
==========================================
+ Hits         3472     3480       +8     
  Misses        613      613              
  Partials      375      375              
Flag Coverage Δ
config-service 98.14% <ø> (ø)
relay 78.63% <ø> (+0.04%) ⬆️
server 83.28% <ø> (ø)
ws-server 36.66% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

@konstantinabl konstantinabl requested a review from a team November 15, 2024 15:29
@natanasow
Copy link
Collaborator

natanasow commented Nov 15, 2024

This design handles the direct precompile call flow well.

There might be a case where we create an HTS token within the contract's constructor. What will we return in that case? Is the newly created contract's address or the HTS token address?

In terms of the Ethereum equivalence, we always must return the contract's address. Maybe we should add a section in the readme with something like "In case of HTS token creation within the constructor, the user should manually emit an event and handle the token address exposure on the contract's side"?

@Nana-EC actually, are we thinking of handling an HTS token creation within a contract and populating the contractAddress field with the newly created token address on the top-level transaction's receipt?

@Nana-EC
Copy link
Collaborator

Nana-EC commented Nov 16, 2024

This design handles the direct precompile call flow well.

There might be a case where we create an HTS token within the contract's constructor. What will we return in that case? Is the newly created contract's address or the HTS token address?

In terms of the Ethereum equivalence, we always must return the contract's address. Maybe we should add a section in the readme with something like "In case of HTS token creation within the constructor, the user should manually emit an event and handle the token address exposure on the contract's side"?

@Nana-EC actually, are we thinking of handling an HTS token creation within a contract and populating the contractAddress field with the newly created token address on the top-level transaction's receipt?

Hmmm, I don't think we need to handle in contract calls as there it is the responsibility of the developer to handle the functions response. And yeah if they want they can emit an event to externalize the token address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design, pilot and prototyping exploration work Internal For changes that affect the project's internal workings but not its outward-facing functionality.
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

Create design doc enabling returning of HTS token address in receipt
3 participants