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

Added data transformation documentation #2555

Open
wants to merge 1 commit into
base: spr/master/9557f1ee
Choose a base branch
from

Conversation

integraledelebesgue
Copy link
Member

@integraledelebesgue integraledelebesgue commented Oct 4, 2024

Comment on lines 106 to 107

It's **not allowed** to write any function call expressions, either builtin, or user-defined.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would make it more verbose, the message of this line isn't obvious to me

call \
--url http://127.0.0.1:5050 \
--contract-address 0x016ad425af4585102e139d4fb2c76ce786d1aaa1cfcd88a51f3ed66601b23cdd \
--function tuple_fn \
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be this

complex_struct_fn

--contract-address 0x016ad425af4585102e139d4fb2c76ce786d1aaa1cfcd88a51f3ed66601b23cdd \
--function tuple_fn \
--calldata \
data_transformer_contract::ComplexStruct { \
Copy link
Member

Choose a reason for hiding this comment

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

We never show the definition of ComplexStruct.

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be NestedStructWithField instead, my fault

call \
--url http://127.0.0.1:5050 \
--contract-address 0x016ad425af4585102e139d4fb2c76ce786d1aaa1cfcd88a51f3ed66601b23cdd \
--function tuple_fn \
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be complex_fn

Comment on lines +146 to +172
### Caveats

Reasoning about calldata serialization logic is not always possible for Cast. **In particular, Cast doesn't verify serialized calldata against the ABI**.

When given a calldata, Cast first tries to transform it according to the contract ABI.
When failed, in case of type or length mismatch, it will then try to interpret it as an already serialized data.

Thus, calldata written in hex-encoded form is not always going to be interpreted as a list of felts and treated as serialized. Let's see an example:

Our `DataTransformerContract` exposes a method with signature:
```rust
fn u256_fn(self: @ContractState, a: u256) { ... }
```

We can write:
```shell
$ sncast --account myuser \
call \
--url http://127.0.0.1:5050 \
--contract-address 0x016ad425af4585102e139d4fb2c76ce786d1aaa1cfcd88a51f3ed66601b23cdd \
--function u256_fn \
--calldata 0x10 \
--block-id latest
```

Despite the call data **not being** the proper serialization of an `u256` (which would be `0x10 0x0`), call succeeds.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should be a bit more strongly worded e.g. explicitly warn users to always use type suffixes etc. so their data doesn't become serialized incorrectly

Copy link
Member

Choose a reason for hiding this comment

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

Also it would probably make sense to add a flag that disables data transformation just to be extra sure. This could be a separate issue and tackled by someone else though

Please note the notation of the argument. The default way of passing calldata is a list of hexadecimally encoded field elements - the *serialized* calldata.
To obtain the serialized form of the wished data, one must write a Cairo program calling `Serde::serialize` on subsequent arguments and displaying the results.

It is also possible to pass calldata in more friendly, human readable form due to the [calldata transformation](./calldata-transformation.md) feature present in Cast.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
It is also possible to pass calldata in more friendly, human readable form due to the [calldata transformation](./calldata-transformation.md) feature present in Cast.
It is also possible to pass calldata in more friendly, human readable form thanks to the [calldata transformation](./calldata-transformation.md) feature present in Cast.

It is also possible to pass calldata in more friendly, human readable form due to the [calldata transformation](./calldata-transformation.md) feature present in Cast.

> ⚠️ **Warning**
> Cast will not werify the serialized calldata. Any errors caused by passing improper calldata in a serialized form will originate from the network.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
> Cast will not werify the serialized calldata. Any errors caused by passing improper calldata in a serialized form will originate from the network.
> Cast will not verify the serialized calldata. Any errors caused by passing improper calldata in a serialized form will originate from the network.

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.

3 participants