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

docs: Add a doc with the decompile/disassembly of the token redirect/proxy … #5673

Merged
merged 7 commits into from
Jan 16, 2025

Conversation

david-bakin-sl
Copy link
Member

@david-bakin-sl david-bakin-sl commented Mar 15, 2023

…contract

Description:

Add a doc with the decompile (into Solidity) and disassembly of the hexstring which is the token redirect/proxy contract.

Related issue(s):

Fixes #17064

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • [N/A] Tested (unit, integration, etc.)

@david-bakin-sl david-bakin-sl requested a review from a team as a code owner March 15, 2023 23:20
@david-bakin-sl david-bakin-sl self-assigned this Mar 16, 2023
@david-bakin-sl david-bakin-sl requested a review from stoqnkpL March 16, 2023 16:45
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 0c14ee0 to 9f2cf1b Compare March 16, 2023 18:34
Copy link
Contributor

@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.

This is good.
I think we can add a little overview section to give context to the token facade and it's usage in follow up from HIP 218.

@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 9f2cf1b to 0156257 Compare March 31, 2023 23:26
@david-bakin-sl david-bakin-sl marked this pull request as draft August 1, 2023 15:10
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 0156257 to 0c21ae1 Compare September 18, 2023 16:57
@david-bakin-sl david-bakin-sl marked this pull request as ready for review September 18, 2023 16:58
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 0c21ae1 to c60fa31 Compare September 18, 2023 16:59
@github-actions
Copy link

Node: E2E Test Results

    1 files      1 suites   20m 6s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit c60fa31.

@github-actions
Copy link

Node: Unit Test Results

    2 200 files      2 200 suites   1h 25m 14s ⏱️
117 934 tests 117 898 ✔️ 36 💤 0
126 253 runs  126 217 ✔️ 36 💤 0

Results for commit c60fa31.

@github-actions
Copy link

Node: Integration Test Results

278 tests   277 ✔️  32m 17s ⏱️
    5 suites      0 💤
    5 files        1

Results for commit c60fa31.

Copy link
Contributor

@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.

This is good.

I think just adding one section before the 3-address with an example of the E2E flow would complete the doc.
Basically having presented and explained the redirect what's an example.
It might look like when a developer creates an HTS token with address 0x0000..1 any an ERC20/ERC721 such as .name() could be called, which would then call the name system contract function name (tokenAddress).
Just an basic illustration would drive this home for a reader.

@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from c60fa31 to 00fe6e5 Compare December 9, 2024 06:53
@david-bakin-sl
Copy link
Member Author

I think just adding one section before the 3-address with an example of the E2E flow would complete the doc. Basically having presented and explained the redirect what's an example. It might look like when a developer creates an HTS token with address 0x0000..1 any an ERC20/ERC721 such as .name() could be called, which would then call the name system contract function name (tokenAddress). Just an basic illustration would drive this home for a reader.

Will add this.

Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.95%. Comparing base (650d911) to head (2907c2e).
Report is 19 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #5673      +/-   ##
============================================
+ Coverage     67.58%   67.95%   +0.36%     
- Complexity    22135    22344     +209     
============================================
  Files          2588     2606      +18     
  Lines         96667    96901     +234     
  Branches      10099    10098       -1     
============================================
+ Hits          65335    65848     +513     
+ Misses        27589    27293     -296     
- Partials       3743     3760      +17     
Files with missing lines Coverage Δ
.../contract/impl/state/DispatchingEvmFrameState.java 89.47% <ø> (ø)

... and 115 files with indirect coverage changes

Impacted file tree graph

Copy link

codacy-production bot commented Dec 9, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.98% (target: -1.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (650d911) 96450 68926 71.46%
Head commit (2907c2e) 95262 (-1188) 69013 (+87) 72.45% (+0.98%)

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 (#5673) 0 0 ∅ (not applicable)

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

@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 00fe6e5 to bb7ad82 Compare December 9, 2024 16:21
@david-bakin-sl david-bakin-sl requested a review from a team as a code owner December 9, 2024 16:21
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from bb7ad82 to f20b5dc Compare December 13, 2024 18:54
@david-bakin-sl david-bakin-sl changed the title Add a doc with the decompile/disassembly of the token redirect/proxy … docs: Add a doc with the decompile/disassembly of the token redirect/proxy … Dec 13, 2024
@david-bakin-sl david-bakin-sl added this to the v0.58 milestone Dec 13, 2024
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 0a8b330 to 61d36ce Compare December 13, 2024 22:11
@Nana-EC
Copy link
Contributor

Nana-EC commented Jan 13, 2025

@david-bakin-sl can we get this updated and rebased for a new round of reviews?

@Nana-EC Nana-EC modified the milestones: v0.58, v0.59 Jan 13, 2025
And ... fix up bytecode hex-constants in `DispatchingEvmFrameState` so they're
in proper units of _bytes_

Signed-off-by: David S Bakin <[email protected]>
Signed-off-by: David S Bakin <[email protected]>
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 61d36ce to 3fe0a5b Compare January 13, 2025 19:41
Signed-off-by: David S Bakin <[email protected]>
@david-bakin-sl david-bakin-sl force-pushed the add-proxy-contract-interpretation branch from 71baf10 to 14f10da Compare January 15, 2025 18:50
Copy link
Contributor

@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.

Looking good, final minor change suggestions

david-bakin-sl and others added 4 commits January 15, 2025 16:34
…ctContract-description.md

Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: David Bakin <[email protected]>
…ctContract-description.md

Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: David Bakin <[email protected]>
…ctContract-description.md

Co-authored-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: David Bakin <[email protected]>
Signed-off-by: David S Bakin <[email protected]>
Copy link
Contributor

@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.

LG

Copy link

@acuarica acuarica left a comment

Choose a reason for hiding this comment

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

lg

@david-bakin-sl david-bakin-sl merged commit f624d4d into main Jan 16, 2025
51 of 52 checks passed
@david-bakin-sl david-bakin-sl deleted the add-proxy-contract-interpretation branch January 16, 2025 02:19
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.

Document in repo how the smart contract service's redirect/proxy contract for system contracts works
4 participants