OpShin / opshin

A simple pythonic programming language for Smart Contracts on Cardano
https://book.opshin.dev/
MIT License
136 stars 26 forks source link

Upgrade to UPLC 1.0.0 #343

Closed nielstron closed 1 month ago

nielstron commented 6 months ago

The latest version of UPLC features a proper compilation pipeline with some optimizations and a fully featured Plutus VM simulator. This would allow estimation of costs for functions, inspecting the trace of programs and integrating the configuration object added in #341.

Bug bounty: 500 ADA

SCMusson commented 1 month ago

This one is actually turning out to be tricky. A few things are simplish:

However quite a few compiled codes are searching for hardcoded things e.g. misc_tests.test_constant_folding_list which has: self.assertIn("(con list<integer> [0, 2, 4, 6, 8])", code.dumps()) but this has changed to be "(con (list integer) [0, 2, 4, 6, 8])". At this point I could try and continue guessing what the correct code should be but I'm getting less sure I'm doing things correct e.g.: (con list<pair<data, data>> [[#4173, #01], [#416d, #00]]) becoming (con (list (pair data data)) [(B #73, I 1), (B #6d, I 0)]). This is turning out be a lot more work than I expected for a 200 ADA bug bounty and I'm concerned I'm not doing things correctly.

Am I on the right track or have I gone astray.

nielstron commented 1 month ago

This upgrade is actually quite a lot of work for "just" a dependency bump, that's right.

The updates in pluthon and uplc should be straightforward. The changes in test function calling is hopefully mostly search and replace. The changes in syntax can be derived by updates to the UPLC syntax that is... difficult to pin down. you can just share the affected cases here and I can let you know how they should look like (the provided examples look correct).

That said, should I raise the bounty to 500 ADA?

SCMusson commented 1 month ago

Raising the bounty would make sense.

I'm currently Running into this error:

Traceback (most recent call last):
  File "/home/wppj21/Workshop/opshin/test_class.py", line 152, in <module>
    ret = eval_uplc_value(source_code, x,z)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/Workshop/opshin/opshin/tests/utils.py", line 51, in eval_uplc_value
    res = eval_uplc(
          ^^^^^^^^^^
  File "/home/wppj21/Workshop/opshin/opshin/tests/utils.py", line 39, in eval_uplc
    raise ret
  File "/home/wppj21/Workshop/uplc/uplc/machine.py", line 111, in eval
    stack.append(self.return_compute(step.context, step.value))
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/Workshop/uplc/uplc/machine.py", line 171, in return_compute
    return self.apply_evaluate(context.ctx, context.val, value)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/Workshop/uplc/uplc/machine.py", line 210, in apply_evaluate
    res = eval_fun(*arguments)
          ^^^^^^^^^^^^^^^^^^^^
  File "/home/wppj21/Workshop/uplc/uplc/ast.py", line 868, in _MkCons
    assert isinstance(
           ^^^^^^^^^^^
AssertionError: Can only cons elements of the same type

With this minimal example:

from opshin.tests.utils import eval_uplc
source_code = f"""
from opshin.prelude import *

@dataclass
class Test(PlutusData):
    CONSTR_ID = 1
    x: int
    y: bytes

def validator(x: int) -> None:
    y = Test(x, b'')
  ''
        """
ret = eval_uplc(source_code, 5)
nielstron commented 1 month ago

There might be a bug in OpShin there: Class constructors should wrap builtins in PlutusData and then create lists of PlutusData. Alternatively this type check maybe doesn't work? I can try running this minimal example with the official uplc binary to figure out if it should be allowed.

https://github.com/OpShin/uplc/blob/8271ff43fa910194ddfc001cb1618a2d78471a93/uplc/ast.py#L868

nielstron commented 1 month ago

I tried running the output of this script in UPLC and did not get an error. I guess there is an issue initializing the sample_element field correctly

nielstron commented 1 month ago

https://github.com/OpShin/uplc/commit/e91683208160cd175dcf198dea419b0709f5cbe9 fixes the reported issue when constructing plutusdata.

SCMusson commented 1 month ago

This has worked.

I am only getting one final error on the unit test tests.test_std.test_integrity.test_integrity_check_list .

https://github.com/OpShin/opshin/blob/b207896eeff86c54047f78b0079dc26311636bee/opshin/tests/test_std/test_integrity.py#L98-L142

gives me:

E assert False == ((True and True)) E + where True = all(<generator object test_integrity_check_list.. at 0x71525dbc60a0>) E + and True = all(<generator object test_integrity_check_list.. at 0x71525db2ec20>)

and the actual error raised when it should be True:

Traceback (most recent call last): File "/home/wppj21/Workshop/opshin/test_class.py", line 163, in eval_uplc(source_code, obj) File "/home/wppj21/Workshop/opshin/opshin/tests/utils.py", line 35, in eval_uplc raise ret File "/home/wppj21/Workshop/uplc/uplc/machine.py", line 111, in eval stack.append(self.return_compute(step.context, step.value)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/wppj21/Workshop/uplc/uplc/machine.py", line 171, in return_compute return self.apply_evaluate(context.ctx, context.val, value) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/wppj21/Workshop/uplc/uplc/machine.py", line 210, in apply_evaluate res = eval_fun(*arguments) ^^^^^^^^^^^^^^^^^^^^ File "/home/wppj21/Workshop/uplc/uplc/ast.py", line 736, in wrapped_fun assert isinstance( ^^^^^^^^^^^ AssertionError: Argument 0 has invalid type, expected type <class 'uplc.ast.PlutusInteger'> got <class 'uplc.ast.BuiltinInteger'> (BuiltinInteger(value=0))

Is this a similar issue?

nielstron commented 1 month ago

I can reproduce the issue and it also fails using the official uplc binary from plutus-core - hence this confirms that there is a bug in the integrity checking of OpShin. I will look into fixing this.

nielstron commented 1 month ago

Should be resolved as of ef1a57c410423acfb0fa0cce3678d36e8cc89e5a

SCMusson commented 1 month ago

That seems to have worked for me. One final thing For this sort of thing: https://github.com/OpShin/opshin/blob/b207896eeff86c54047f78b0079dc26311636bee/opshin/tests/test_misc.py#L1317

I have been changing to: res = uplc_eval(code, uplc.PlutusInteger(0)) Since it seems that uplc.eval_uplc now does the apply itself.

All tests are working on my PC and I've pushed a requirement in the pull request for opshin to use uplc>=1.0.6. I think the only remaining tasks are: Pluthon pull request for updating uplc will need merging. Finally the CI won't work until uplc 1.0.6 and a Pluthon version with updated uplc requirements is published on PyPi correct?

nielstron commented 1 month ago

The pypi versions should be up to date now