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

Add static type for Cadence external PathLink value #2167

Closed
fxamacker opened this issue Nov 25, 2022 · 4 comments · Fixed by #2248 or #2401
Closed

Add static type for Cadence external PathLink value #2167

fxamacker opened this issue Nov 25, 2022 · 4 comments · Fixed by #2248 or #2401
Assignees

Comments

@fxamacker
Copy link
Member

fxamacker commented Nov 25, 2022

Issue To Be Solved

cadence.PathLink doesn't have a static type. It is needed because:

  • CCF specs (draft) needs to include cadence.PathLink static type.
  • CCF codec (WIP) needs all Cadence external values to have a static type.

Updates #2157

Suggested Solution

Maybe add PathLinkType and return PathLinkType from PathLink.Type().

EDIT: Renamed Link to its new name PathLink.

@fxamacker fxamacker changed the title Add static type for cadence external Link value Add static type for Cadence external Link value Dec 11, 2022
@turbolent turbolent removed their assignment Dec 16, 2022
@fxamacker fxamacker changed the title Add static type for Cadence external Link value Add static type for Cadence external PathLink value Dec 18, 2022
@SupunS SupunS self-assigned this Jan 10, 2023
@turbolent
Copy link
Member

I might have commented on this somewhere before, but I don't think that this is actually needed:
After some digging, support for exporting links was implemented very early on and is/was likely only needed for the Playground. I can't think of a way when a link would be exported, as links are stored internally, but are not first-class values that could be imported or exported by a user.

@SupunS
Copy link
Member

SupunS commented Jan 13, 2023

I initially had the same thought, but would this be needed in the future? e.g: would the CCF be used for encoding cadence internal data (e.g: elaboration serialization, etc.)?

@SupunS
Copy link
Member

SupunS commented Mar 17, 2023

Reopening the issue, since the removal of PathLink value was reverted: #2381.

This would still be a problem for CCF.

@SupunS SupunS reopened this Mar 17, 2023
@turbolent
Copy link
Member

I followed up with the DX team: The Emulator provides the functionality to export values from account storage, and the Playground API in turn encodes the values further to JSON-CDC.

That means that we'll want to keep the export functionality (interpreter.Value to cadence.Value), and the JSON-CDC encoding, but can remove import related code.

In addition, we should add support for account link values.

For CCF, we can just ignore links.

I'll open a PR for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants