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

🐛 Bug ➾ optional parameter on entrypoint not correctly generated by Taqueria makes Typescript wrong #1861

Closed
1 of 6 tasks
zamrokk opened this issue Apr 3, 2023 · 8 comments · Fixed by #1873
Closed
1 of 6 tasks
Assignees
Labels
bug Something isn't working triage Requires triage

Comments

@zamrokk
Copy link

zamrokk commented Apr 3, 2023

🚥 Status (Internal Taqueria Team Use Only)

  • 🔬 Investigated and Verified
  • ⚗️ Solution Identified and Designed
  • 🧫 Dev Implementation of Fix
  • 🧪 Fix Tested and Validated
  • 🏆 PR Merged

🆘 What happened?

image

My entrypoint contains an optional, when I run the taqueria type generation it creates the fields in random postiion, so my optional field here appear on first position. On Typescript, we should list all mandatory fields first

Here the parameter in jsligo from the smart contract :

export type removeMemberRequest = { member : address, orgName : string, lastAdmin : option<address> };

Here the portion of the file generated by Taqueria

type Methods = { removeMember: ( lastAdmin?: address, member: address, orgName: string, ) => Promise<void>; ...

🆘 Steps to Reproduce?

add on optionla field on an entrypoint parameter and generate the type via taqueria plugin

🪵 Relevant log output

No response

🐘 How impactful is this bug?

🤔 Notable but managable

⏱️ Prevalance

No response

💻 Operating System

None

🕸️ System Architecture

None

🌮 Taqueria Version

v0.28.4

🌿 Node.js Version

No response

☎️ Contact Details

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@zamrokk zamrokk added bug Something isn't working triage Requires triage labels Apr 3, 2023
@zamrokk
Copy link
Author

zamrokk commented Apr 12, 2023

asking ligo team about WHO is deciding (alphabetic) field ordering in Michelson ...

image

@zamrokk
Copy link
Author

zamrokk commented May 3, 2023

@michaelkernaghan Is it possible to put high priority on this ticket ? It is just impossible to pass any call to the smart contract if it contains an entrypoint with option

I am totally stuck right now, because I cannot trick the type definition in main.types.ts, if I do this, when taquito will pass the methods arguments, it will be in different order than the one expected by the smart contract and always fails.

This is more than annoying, because in my case, I cannot cheat with an empty string to simulate an empty value, the Tezos node is expecting an address type

image

I could not find a workaround

Thanks

@zamrokk
Copy link
Author

zamrokk commented May 3, 2023

Ok, I found a woraround, maybe it is also the way to fix it quickly

Keep same order, but change how Typescript generates the type definition

Do this

lastAdmin: address | undefined,

Instead of

lastAdmin?: address,

@mweichert @michaelkernaghan

Can you do this fix ?
I tried with taquito passing undefined and an address and both seems ok for the node

On tzkt indexer, it says when undefined is passed that the value on storage is null, so I suppose is the representation of None()

@zamrokk
Copy link
Author

zamrokk commented May 25, 2023

I saw you choose the reorder the fields, I am not sure it is working as Michelson contract field ordering has to be the same on the node side ...
You should do a test calling a contract

I would recommend replace myfield? by myField | null

@zamrokk
Copy link
Author

zamrokk commented Jun 27, 2023

@michaelkernaghan @mweichert For me the bug is still here. We should reopen this ticket as I need to manually process all generated files

I recommend to fix this with myField | null instead of myField?

@mweichert
Copy link
Collaborator

OK, thanks for the additional feedback, @zamrokk. I had tested this but perhaps the contract I was using was not sufficient for this case.

Btw, Michael K and others from the ECAD Labs team are no longer working on Taqueria.

@mweichert mweichert reopened this Jun 27, 2023
@mweichert
Copy link
Collaborator

ecadlabs/taquito#2344

@github-actions
Copy link
Contributor

Published Binaries & Packages

Published Version
Taq Binary (MacOS) 0.36.9
Taq Binary (Windows) 0.36.9
VSIX for VSCode Extension 0.36.9
@taqueria/plugin-archetype 0.36.9
@taqueria/plugin-contract-types 0.36.9
@taqueria/plugin-core 0.36.9
@taqueria/plugin-flextesa 0.36.9
@taqueria/plugin-ipfs-pinata 0.36.9
@taqueria/plugin-jest 0.36.9
@taqueria/plugin-ligo 0.36.9
@taqueria/plugin-metadata 0.36.9
@taqueria/plugin-smartpy 0.36.9
@taqueria/plugin-taquito 0.36.9
@taqueria/plugin-tezos-client 0.36.9
@taqueria/protocol 0.36.9
@taqueria/node-sdk 0.36.9
@taqueria/toolkit 0.36.9

Note: You can install the latest development version of a package with taq install @taqueria/[packageName]@dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Requires triage
Projects
None yet
3 participants