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

Contract API tests #42

Merged
merged 11 commits into from
Aug 29, 2023
Merged

Contract API tests #42

merged 11 commits into from
Aug 29, 2023

Conversation

pmikolajczyk41
Copy link
Member

Adding tests for ContractAPI. Also, bump version to 1.3 - it should be now ready to fill the gaps in ink!

@pmikolajczyk41 pmikolajczyk41 requested a review from deuszx August 28, 2023 14:57
@@ -21,5 +21,8 @@ parity-scale-codec = { workspace = true }
scale-info = { workspace = true }
thiserror = { workspace = true }

[dev-dependencies]
wat = { workspace = true }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wat is das?

Copy link
Member Author

Choose a reason for hiding this comment

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

can turn wat to a wasm blob 🤷‍♂️

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you have to write a wat by hand?

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. pallet-contracts' unit tests also use wat instead of full Rust contract compiled to wasm
  2. no need for compiling contract to wasm / no need to keep obscure bytes
  3. kinda readable
  4. you don't have to encode any selectors or arguments: data: vec![] works well for calling the only method

(i32.store
(i32.const 40)
(call $seal_transfer
(i32.const 0) ;; ptr to destination address
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using zero-address is incorrect as the private key for it is well-known ;P

Doesn't matter in this context of course xD

Copy link
Member Author

Choose a reason for hiding this comment

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

copied from Substrate :P

Copy link
Collaborator

Choose a reason for hiding this comment

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

good practices etc :P

Copy link
Member Author

Choose a reason for hiding this comment

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

removed transferring at all (it was silently failing due to insufficient contract's balance)

Copy link
Collaborator

@deuszx deuszx Aug 28, 2023

Choose a reason for hiding this comment

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

and tests were passing? How's that po(a)ssible? ie how was the event emitted if the txn failed? Or at this level the event emition is not yet affecting what ends up in the final block?

Copy link
Member Author

Choose a reason for hiding this comment

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

okay, taking that back - I have no idea why it was passing

fn can_upload_code() {
let mut sandbox = Sandbox::<MinimalRuntime>::new().unwrap();
let wasm_binary = compile_module("transfer");
let hash = <<MinimalRuntime as frame_system::Config>::Hashing>::hash(&wasm_binary);
Copy link
Collaborator

Choose a reason for hiding this comment

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

<3

Base automatically changed from events to main August 29, 2023 06:54
@pmikolajczyk41 pmikolajczyk41 merged commit 27aa56e into main Aug 29, 2023
@pmikolajczyk41 pmikolajczyk41 deleted the contract-api-tests branch August 29, 2023 07:09
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