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

fix: catch instantiate reversion #512

Merged
merged 2 commits into from
Sep 4, 2023
Merged

Conversation

statictype
Copy link
Contributor

Inspect the flags and disallow instantiation in case a reversion took place

closes #494

@netlify
Copy link

netlify bot commented Aug 28, 2023

Deploy Preview for contracts-ui ready!

Name Link
🔨 Latest commit 309ab54
🔍 Latest deploy log https://app.netlify.com/sites/contracts-ui/deploys/64ef3b27c690050008e5527d
😎 Deploy Preview https://deploy-preview-512--contracts-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@statictype statictype changed the title Catch instantiate reversion feat: catch instantiate reversion Aug 28, 2023
@cypress
Copy link

cypress bot commented Aug 28, 2023

Passing run #208 ↗︎

0 61 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

naming is hard
Project: Contracts UI Commit: 309ab541d0
Status: Passed Duration: 03:20 💡
Started: Aug 30, 2023 12:53 PM Ended: Aug 30, 2023 12:57 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@statictype statictype changed the title feat: catch instantiate reversion fix: catch instantiate reversion Aug 29, 2023
peetzweg
peetzweg previously approved these changes Aug 29, 2023
Copy link
Collaborator

@peetzweg peetzweg left a comment

Choose a reason for hiding this comment

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

Lgtm. Was a bit confused about the word reversion but seems to be the correct one to use here. Maybe checkInstantiateReverted & hasReverted? Fine either way. 👍


export function checkInstantiateReversion(dryRunResult: ContractInstantiateResult | undefined) {
return dryRunResult?.result.isOk
? dryRunResult.result.asOk.result.flags.toHuman().includes('Revert')
Copy link
Collaborator

@peetzweg peetzweg Aug 29, 2023

Choose a reason for hiding this comment

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

dryRunResult.result.asOk.result.flags has no field/flag hasReverted or so?

Not familiar pjs to deeply still and can't find anything in the docs. Not sure if this is somehow available. Just did fire and forget search before our meeting.

https://github.com/polkadot-js/api/blob/ab3b162d7eb96cccef9e3ff057d4dbe96ffe01a3/packages/types/src/interfaces/contracts/types.ts#L182

@statictype statictype merged commit eacc914 into master Sep 4, 2023
11 checks passed
@statictype statictype deleted the ae-instantiate-reversion branch September 4, 2023 13: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.

Instantiate dry-run doesn't catch the reversion
2 participants