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

Use tgrade custom in valset #182

Merged
merged 5 commits into from
Sep 29, 2021
Merged

Use tgrade custom in valset #182

merged 5 commits into from
Sep 29, 2021

Conversation

ethanfrey
Copy link
Contributor

Builds on #180

This does a proper suite setup to use the Tgrade app, and register privileged contracts.
Does proper begin/end block calls to act like a real runtime.

ueco-jb
ueco-jb previously approved these changes Sep 29, 2021
Copy link

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

Nitpicking comments, lgtm

@@ -41,5 +41,7 @@ cosmwasm-schema = { version = "1.0.0-soon" }
cw-multi-test = { version = "=0.10.0-soon3" }
tg4-engagement = { path = "../tg4-engagement", version = "0.4.0" }
tg4-stake = { path = "../tg4-stake", version = "0.4.0" }
# we enable multitest feature only for tests
Copy link

Choose a reason for hiding this comment

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

IMO this comment is redundant.

Comment on lines 1527 to 1530
suite.app().update_block(|block| {
block.height += 20;
block.time = block.time.plus_seconds(100);
});
Copy link

Choose a reason for hiding this comment

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

I think this 5s/block ratio should be somehow/somewhere standarized (some constant in utils to calculate?).
I see it from time to time, but without context it may be weird to see that you increment height by seemingly arbitral 20.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is semi-codified here: https://github.com/CosmWasm/cw-plus/blob/main/packages/multi-test/src/app.rs#L23-L26

and based on typical block times on Cosmos chains. It would be good to make this a const

@ethanfrey ethanfrey mentioned this pull request Sep 29, 2021
11 tasks
Base automatically changed from tgrade-custom-multitest to main September 29, 2021 10:41
ueco-jb
ueco-jb previously approved these changes Sep 29, 2021
Copy link

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

lgtm

@ethanfrey
Copy link
Contributor Author

One more fix for your comment on magic block times

Copy link

@ueco-jb ueco-jb left a comment

Choose a reason for hiding this comment

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

lgtm

block.height += 20;
block.time = block.time.plus_seconds(100);
});
suite.app().advance_seconds(100);
Copy link

Choose a reason for hiding this comment

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

👍

@hashedone hashedone merged commit 294b01a into main Sep 29, 2021
@hashedone hashedone deleted the use-tgrade-custom-in-valset branch September 29, 2021 11:58
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