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

CallReceipt is returning wrong from property #2449

Closed
LuizAsFight opened this issue Jun 6, 2024 · 11 comments · Fixed by #3385
Closed

CallReceipt is returning wrong from property #2449

LuizAsFight opened this issue Jun 6, 2024 · 11 comments · Fixed by #3385
Assignees
Labels
bug Issue is a bug

Comments

@LuizAsFight
Copy link
Contributor

LuizAsFight commented Jun 6, 2024

Motivation

Contract Call receipts are getting a from property
I think from is not correct for this receipt. It should be id like in other case inside that same switch, or it should be removed.

reasons according to spec :
1- shows Call contracts prop already have a from prop
2- there is no contractId and no id in CallReceipt, which means the from being provided by this method will be always 0x0000...

UPDATE

To fix this, we need to ensure that the TS SDK's receipts types are in alignment with the Fuel specs

Spec changes

  • ReceiptCall
    *
@arboleya arboleya added bug Issue is a bug triage labels Jun 9, 2024
@petertonysmith94
Copy link
Contributor

Summary

  • I agree with this assessment that the from property should be changed to id.
  • It seems like the from property isn't even returned - I do question if the spec's are also off?
  • I also looked over the Transfer and TransferOut and they also have a similar issue (from what I understand).

Call

Transfer

TransferOut

@petertonysmith94 petertonysmith94 self-assigned this Jun 12, 2024
@petertonysmith94
Copy link
Contributor

@danielbate you mentioned in sync about other possible mismatches?

@petertonysmith94 petertonysmith94 added this to the 0.x post-launch milestone Jun 17, 2024
@danielbate
Copy link
Member

I believe #2530 and #2448 are related.

@petertonysmith94 petertonysmith94 removed their assignment Jun 17, 2024
@arboleya arboleya added p2 and removed p1 labels Jul 19, 2024
@arboleya arboleya removed this from the 0.x post-launch milestone Jul 19, 2024
@arboleya arboleya added p1 and removed p2 labels Jul 19, 2024
@Torres-ssf
Copy link
Contributor

Torres-ssf commented Jul 22, 2024

@LuizAsFight there is no error here.

We cannot rename from to id on the CallReceipt type as on the specs for this receipt the property name is from and not id.

Also, the property from does not exist within the raw receipt type returned by fuel-core.

That is why its value is being extracted from id.

The value for from will be null/undefined most of the time. Just like it is said within the specs:

from: Hexadecimal string representation of the 256-bit (32-byte) contract ID of the current context if in an internal context; null otherwise.

This property will only be defined in case of a contract that is calling another contract. When this is not the case, it will be null. That is why in most cases its value is 0x0000...

@Torres-ssf Torres-ssf added p2 and removed p1 labels Jul 25, 2024
@arboleya arboleya removed the p2 label Aug 2, 2024
@arboleya
Copy link
Member

arboleya commented Aug 8, 2024

@Torres-ssf Can this be closed?

@Torres-ssf
Copy link
Contributor

@arboleya I am not enterely sure.

I may have misunderstood @LuizAsFight statement, so I am waiting for his confirmation

@arboleya arboleya added the temp-linear label Sep 8, 2024 — with Linear
Copy link
Member

@LuizAsFight Gentle ping — any input here?

@LuizAsFight
Copy link
Contributor Author

LuizAsFight commented Sep 12, 2024

@arboleya @Torres-ssf

the problem here is returning things different from spec.

for instace this is what the spec says about "from" field in Call receipts:

from: Hexadecimal string representation of the 256-bit (32-byte) contract ID of the current context if in an internal context; null otherwise.

and this is what the sdk returns from "from" field

from: hexOrZero(receipt.id || receipt.contractId),

problem 1: "from" returned by sdk doesn't reflect the real "from" from spec
problem 2: "from" is not returned anywhere inside of callReceipt, so users don't have access to it
problem 3: "contractId" doesn't exist on spec for Call receipt
problem 4: "id" doesn't exist on spec for Call receipt
problem 5: the union of problem 3 and 4, makes the field "from" be returned always as zero (0x) in current implementation


Instead of returning the above I believe the sdk should return it parsed but don't manipulate the fields, something like:

from: hexOrZero(receipt.from),

I think all receipt types should be visited to check if this happens also, and then fix them

@Torres-ssf
Copy link
Contributor

@LuizAsFight I am closing this as it was addressed on FuelLabs/fuel-specs#610

Copy link
Contributor

@Torres-ssf should we be closing this one? I'd of thought we'd need to sync with the specification?

For my understanding of the spec PR, it now appears that we're not aligned with the specification:

  • e.g. ReceiptCall in the spec has changed from from to id , and we still maintain this as from (here).

@Torres-ssf
Copy link
Contributor

@Torres-ssf should we be closing this one? I'd of thought we'd need to sync with the specification?For my understanding of the spec PR, it now appears that we're not aligned with the specification:

  • e.g. ReceiptCall in the spec has changed from from to id , and we still maintain this as from (here).

@petertonysmith94 You are correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants