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

Add validation of smart contract calls #802

Merged

Conversation

netboz
Copy link
Contributor

@netboz netboz commented Jan 4, 2023

Description

Provide an endpoint to pre-validate smart contract execution

Fixes #57

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@netboz netboz added smart contracts Involve smart contracts transaction Involve transactions API Involve API facing user core team Assigned to the core team enhancements labels Jan 4, 2023
lib/archethic/contracts.ex Outdated Show resolved Hide resolved
lib/archethic/contracts.ex Outdated Show resolved Hide resolved
@netboz netboz force-pushed the pr_57_validate_smart_contract_calls branch from 17ac581 to 58e6a69 Compare January 5, 2023 10:58
@bchamagne
Copy link
Member

When I go to localhost:4000/api/transaction/contract/simulator I get this error:

Screenshot 2023-01-12 at 15-20-17 Phoenix ActionClauseError at GET _api_transaction_contract_simulator

@netboz netboz self-assigned this Jan 25, 2023
@netboz netboz force-pushed the pr_57_validate_smart_contract_calls branch from 278b185 to 2ee2886 Compare January 25, 2023 16:53
lib/archethic/contracts.ex Outdated Show resolved Hide resolved
@netboz netboz force-pushed the pr_57_validate_smart_contract_calls branch from 299c122 to 61b2ced Compare January 29, 2023 21:09
Copy link
Member

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

Here's a few comments (some very biased) based only on reading the code. I'm going to need your help for a real word test.

lib/archethic.ex Outdated Show resolved Hide resolved
Archethic.simulate_contract_execution!(prev_tx, tx)

{:error, reason} ->
{false, reason}
Copy link
Member

Choose a reason for hiding this comment

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

It look like this {false, _} is not handled in the Stream.map? Did you mean a %{valid: false, ...} struct?

lib/archethic/contracts.ex Outdated Show resolved Hide resolved
@netboz netboz force-pushed the pr_57_validate_smart_contract_calls branch 4 times, most recently from 32ed025 to cfa4fc6 Compare February 15, 2023 13:48
Copy link
Member

@bchamagne bchamagne left a comment

Choose a reason for hiding this comment

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

Works great!

Tested with the transaction builder & curl.
I tested 3 scenarios:

valid contract:
[{"address":"00002223BBD4EC3D64AE597696C7D7ADE1CEE65C639D885450AD2D7B75592AC76AFA","valid":true}]

invalid contract:
[{"address":"00002223BBD4EC3D64AE597696C7D7ADE1CEE65C639D885450AD2D7B75592AC76AFA","reason":"invalid_inherit_conditions","valid":false}]

exception throwing contract:
[{"reason":"A contract exited while simulating","valid":false}]

I found an issue with the transaction condition, but this is not the goal of this PR, I'll create an issue for that.

ps: maybe add the address to the exit message for consistency?

@Neylix Neylix marked this pull request as draft February 27, 2023 15:23
@bchamagne
Copy link
Member

We are not going to merge that one yet, we will integrate it as soon as interpreter v1 is merged. We want to check the conditions in the Interpreter.exectute functions directly. Then we can use Interpreter.execute directly in the validate_contract function and in the worker as well.

@bchamagne bchamagne force-pushed the pr_57_validate_smart_contract_calls branch from 6886076 to 48bcf6c Compare March 15, 2023 18:11
@samuelmanzanera samuelmanzanera marked this pull request as ready for review March 16, 2023 08:44
incoming_tx,
date
) do
case valid_from_trigger?(trigger_type, incoming_tx, date) do
Copy link
Member

Choose a reason for hiding this comment

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

I think a if would do the job instead of a case ;)

Copy link
Member

Choose a reason for hiding this comment

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

you could even use a with

@bchamagne bchamagne marked this pull request as draft March 16, 2023 09:02
@bchamagne
Copy link
Member

bchamagne commented Mar 16, 2023

I'm going to do one last thing on this PR:

  • move the Contracts.simulate_contract_execution into Interpreter.execute. And then I'm going to use from both the simulate endpoint and from the worker (the only difference is the worker will actually persist the next transaction).

@bchamagne bchamagne marked this pull request as ready for review March 17, 2023 17:53
@bchamagne bchamagne force-pushed the pr_57_validate_smart_contract_calls branch from 2bde966 to d47a43c Compare March 17, 2023 17:58
lib/archethic.ex Outdated Show resolved Hide resolved
@bchamagne bchamagne force-pushed the pr_57_validate_smart_contract_calls branch from 4cac2d8 to bbe896d Compare March 27, 2023 13:32
Copy link
Member

@samuelmanzanera samuelmanzanera left a comment

Choose a reason for hiding this comment

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

Code is nicer ;)

@Neylix Neylix merged commit 8725d25 into archethic-foundation:develop Mar 31, 2023
@samuelmanzanera samuelmanzanera added feature New feature request and removed enhancements labels May 2, 2023
@samuelmanzanera samuelmanzanera changed the title Pr 57 validate smart contract calls Add validation of smart contract calls May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Involve API facing user core team Assigned to the core team feature New feature request smart contracts Involve smart contracts transaction Involve transactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validate smart contract calls
6 participants