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: document struct coded and add examples #406

Merged
merged 6 commits into from
Jun 29, 2022

Conversation

mattsse
Copy link
Member

@mattsse mattsse commented Jun 28, 2022

document struct ABI codec and add examples to cast (sig, send)

ref foundry-rs/foundry#2151

@onbjerg
Copy link
Member

onbjerg commented Jun 28, 2022

I think the struct encoding file needs to be in summary.md to be rendered

@mattsse
Copy link
Member Author

mattsse commented Jun 28, 2022

moved to new misc section

@mattsse
Copy link
Member Author

mattsse commented Jun 28, 2022

the error does not make any sense to me....

@onbjerg
Copy link
Member

onbjerg commented Jun 28, 2022

Me neither, maybe it's because of the extra newline before the code block in the examples list? 🤔

@mattsse
Copy link
Member Author

mattsse commented Jun 28, 2022

was the line indent lol

@gakonst
Copy link
Member

gakonst commented Jun 28, 2022

Can we also make it work so that people can copy/paste struct definitions in line? cast call 0x1234 struct Foo { address x; int256 y; } or do we think it doesn't make sense? Have seen ppl get confused by the tuple syntax

Copy link
Member

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

Got some structural nits/phrasing but also have one comment for one of the examples. Will just batch commit the nits except for the one I'm unsure about

src/misc/README.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
src/misc/struct-encoding.md Outdated Show resolved Hide resolved
Comment on lines 67 to 71
Structs are encoded as tuples, See [struct encoding](./reference/common/struct-encoding.md)

```sh
cast send --ledger 0x... "myfunction((address,uint256))" 0x... 1
```
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
Structs are encoded as tuples, See [struct encoding](./reference/common/struct-encoding.md)
```sh
cast send --ledger 0x... "myfunction((address,uint256))" 0x... 1
```
Structs are encoded as tuples (see [struct encoding](./reference/common/struct-encoding.md))
```sh
cast send --ledger 0x... "myfunction((address,uint256))" 0x... 1
```

Also, would this not be e.g. cast send "myfunction((address,uint256))" 0x... "(0x..., 1)"?

Copy link
Member Author

Choose a reason for hiding this comment

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

USAGE:
    cast send [OPTIONS] <TO> [ARGS]

ARGS:
    <TO>
            The destination of the transaction.

    <SIG>
            The signature of the function to call.

    <ARGS>...
            The arguments of the function to call.

so cast send <to> <sig> <args> is correct here

Copy link
Member

Choose a reason for hiding this comment

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

Ye but args in this case would be "(0x..., 1)". The current example is:

cast send --ledger 0x... "myfunction((address,uint256))" 0x... 1

In other words:

Send using Ledger addr 0x..., call "myfunction" at address 0x... with argument 1?

Copy link
Member Author

@mattsse mattsse Jun 29, 2022

Choose a reason for hiding this comment

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

guess that makes sense, I only copy pasted the other example tbh

src/reference/cast/cast-sig.md Outdated Show resolved Hide resolved
onbjerg and others added 2 commits June 29, 2022 08:35
Co-authored-by: Bjerg <onbjerg@users.noreply.github.com>
@onbjerg
Copy link
Member

onbjerg commented Jun 29, 2022

Can merge when the base PR is merged

@onbjerg onbjerg merged commit dd6b108 into foundry-rs:master Jun 29, 2022
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