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

Catch error in proof of work to get contracts parsing error #628

Merged
merged 15 commits into from
Dec 2, 2022

Conversation

samuelmanzanera
Copy link
Member

Description

This PR resolves an issue occurred when you are passing invalid smart contract code to the network.
The Proof of work was trying to parse the contract to get the origin family, but without catching the error.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Create a transaction with invalid keyword in the contract, it should return an invalid transaction with the error returned as "unexpected-token - xxxx - LX"

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

@samuelmanzanera samuelmanzanera added smart contracts Involve smart contracts mining Involve transaction validation and mining labels Oct 13, 2022
@samuelmanzanera samuelmanzanera self-assigned this Oct 13, 2022
@apoorv-2204
Copy link
Contributor

we might have some test cases?

@Neylix Neylix marked this pull request as draft October 18, 2022 11:35
@samuelmanzanera samuelmanzanera added the core team Assigned to the core team label Oct 27, 2022
@samuelmanzanera samuelmanzanera force-pushed the fix_mining_sc_error branch 7 times, most recently from cd2e7ce to 03a8728 Compare November 23, 2022 16:14
@samuelmanzanera samuelmanzanera marked this pull request as ready for review November 23, 2022 21:23
@samuelmanzanera samuelmanzanera requested review from apoorv-2204 and removed request for apoorv-2204 November 24, 2022 08:46
@Neylix
Copy link
Member

Neylix commented Nov 30, 2022

Hello,
It's not part of your change, but you can fix a bug in this PR.
In the Worker module, the function handle_new_transaction broadcast the message StartMining but it do not do the validation node election before. This cause an invalid node election when other node start mining.
This function should use Archethic.send_new_transaction(tx)

@samuelmanzanera samuelmanzanera merged commit 0d5fbd6 into develop Dec 2, 2022
@samuelmanzanera samuelmanzanera deleted the fix_mining_sc_error branch December 2, 2022 16:25
tenmoves pushed a commit to tenmoves/archethic-node that referenced this pull request Dec 6, 2022
To simplify the interpreter, it is now built of two interpreters:
* Add Condition's interpreter
* Add Action's interpreter

Hence, the validation is easier to handle and to customize. 

This PR brings other improvements:
* Update interpreters
* Uniform the binary/hex handling in smart contract
* Fix SC call inputs serialization
* Clean inputs ETS table for SC calls
* Clear pending transactions table

Co-authored-by: Neylix <julien.leclerc05@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core team Assigned to the core team mining Involve transaction validation and mining smart contracts Involve smart contracts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants