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

feat: showcase available storage types #15

Closed
wants to merge 13 commits into from
Closed

Conversation

peetzweg
Copy link
Contributor

@peetzweg peetzweg commented Apr 3, 2023

Adds first draft of a contract to showcase all types which can be stored. Not an ink/rust expert so happy to get some review what to improve.

Types:

  • u[8,16,32,64,128]
  • i[8,16,32,64,128]
  • struct
  • enum
  • bool
  • Hash
  • Balance
  • AccountId
  • Tuple
  • Array
  • Vec (ink-prelude)
  • String (ink-prelude)
  • Option Some
  • Option None
  • Result Ok
  • Result Error
  • Mapping (ink-storage)
  • Custom data structure with #[ink::storage_item]

What else is available?

Even more types available via prelude => https://docs.rs/ink_prelude/latest/ink_prelude/

But afaik they are more useful for keeping internal contract state and are not exposed as returned values.

  • HashMap (ink-prelude)
  • BTreeMap (ink-prelude
  • Lazy (ink-storage)
  • ...

Origin:

We want such a contract to create some e2e tests showcasing all possible types available in ink for the contracts-ui. Related PR: use-ink/contracts-ui#449 .

@peetzweg peetzweg requested a review from statictype April 3, 2023 10:13
@statictype
Copy link

there are also Option, Tuple, Array, bool

@peetzweg peetzweg requested a review from SkymanOne April 17, 2023 06:59
@peetzweg
Copy link
Contributor Author

peetzweg commented Apr 17, 2023

Hey @SkymanOne, can you have a look at this? Does this look okay'ish to you? Can it be a bit simpler somehow or other recommendations?

Is there a way to put mappings into a struct in ink?

https://github.com/paritytech/ink-examples/pull/15/files#diff-a89a9d0d60bbad817daaa9d021e559bd1142ef3c6b8fe02001b98a2c49e310b3R119-R122

I get an error along theses lines if I put a mapping in a struct with the same annotation as the others structs in the contract.

Screenshot 2023-04-17 at 11 55 53

@peetzweg peetzweg changed the title feat: showcase available types feat: showcase available storage types Apr 17, 2023
@peetzweg
Copy link
Contributor Author

@statictype what do you think about this? I would consider this complete enough. Afaik it covers all types meant to be returned by contracts via messages. Stuff like Mappings and BTreeMap etc. from prelude don't seem built to be returned by ink messages and just for internal contract data.

If we call it complete I would add this contract as a e2e test to contracts-ui.

@peetzweg
Copy link
Contributor Author

peetzweg commented Apr 19, 2023

Okay just realized this PR should be into the ink repo itself. ed91979

The more I learn about rust and ink, the more I can appreciate the mother contract. Not to convinced off the use of the contract made in this pr anymore. Regarding giving complex storage examples this PR looks a bit more promising even. use-ink/ink#1592

What do you think about this @statictype ?

@statictype
Copy link

haven't compiled it yet but LGTM! One thing that is missing is something like revertOrTrap in the mother contract. it returns an error object (as opposed to error strings). We can add this later, your call.
It should live in the integration tests folder, indeed. We only use it for testing, not an actual example.

The more I learn about rust and ink, the more I can appreciate the mother contract. Not to convinced off the use of the contract made in this pr anymore

why? the mother contract was incomplete in terms of types and not very specific in terms of naming.

@peetzweg peetzweg removed the request for review from SkymanOne April 20, 2023 08:24
@peetzweg
Copy link
Contributor Author

@statictype just added a get_panic message which just panics and together with the get_result_error message it should be the equivalent of the revert_or_trap function of the mother contract.

https://github.com/paritytech/ink-examples/blob/40fbf9d8082174ae11eed87482ed66d167125fec/storage_types/lib.rs#L235-L245

revert_or_trap in mother:
https://github.com/paritytech/ink/blob/bc4786ed4b2007d4b821aea40abb734373477266/integration-tests/mother/lib.rs#L172-L184

What I like about the mother contract is the fact that it's function just echo the input data. But this kind of functionality is maybe nice to have in a separate contract to test the sending of extrinsics from the contracts-ui. Whereas this contract is just focused on decoding the available return types.

Will create the pr for the ink repo now.

@peetzweg
Copy link
Contributor Author

Closed in favor of use-ink/ink#1762.

@peetzweg peetzweg closed this Apr 20, 2023
peetzweg added a commit to use-ink/ink that referenced this pull request Nov 16, 2023
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.

2 participants