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

CashScript debugger (@mainnet-pat's flipstarter) #172

Open
rkalis opened this issue Nov 22, 2023 · 23 comments
Open

CashScript debugger (@mainnet-pat's flipstarter) #172

rkalis opened this issue Nov 22, 2023 · 23 comments

Comments

@rkalis
Copy link
Member

rkalis commented Nov 22, 2023

@mainnet-pat is working on a debugger for CashScript. For this he is making certain changes to the CashScript compiler and SDK. We'll use this issue to document the progress on that.

This issue is related (and solves) the following long-standing issues as well:
#72
#76

@rkalis rkalis changed the title CashScript debugger (@mainnet-pat's flipstarter CashScript debugger (@mainnet-pat's flipstarter) Nov 22, 2023
@rkalis
Copy link
Member Author

rkalis commented Nov 22, 2023

pat, mathieu and I had a call today to discuss some of these things.

pat has been working on integrating source mapping into the CashScript compiler. He's doing that by keeping track of the AST's Location data in the GenerateBytecodeTraversal. This sourcemap information is represented in the same format that Solidity uses for its source mapping and is included in the artifact output.

To make it work for now, we're also adding a "unoptimised bytecode" output that will be used for debugging purposes, because it is much simpler to apply the source-mapping to that compared to the optimised bytecode. As an extension it may be interesting to make it work properly with optimised bytecode as well.

To use this sourcemap, we also require a Libauth template to be exported from CashScript so that it can be evaluated through libauth (and combined with the sourcemap). This is related to #72 and #76. For now this will be implemented in the cashscript library at first, but might be good to create a separate library for in the future. This evaluation can then also be used to write automated tests for your smart contracts that can execute in a Libauth evaluation environment.

pat is also working on adding console.log and require(x, "Failure Message") syntax, which requires the evaluation library (definitely Libauth, but we could try to extend this to node software as well) to keep track of an "instruction pointer" and return it when the transaction is rejected, so we can map the instruction pointer to the corresponding message.

@rkalis
Copy link
Member Author

rkalis commented Jan 3, 2024

Mathieu and I did some refactoring and adding of tests today, both in the utils and the cashc packages.

Next time, we want to refactor utils/source-map.ts and the CashScript compiler (especially GenerateTargetTraversal). After that we can move on to the most important parts: the libauth template generation.

@rkalis
Copy link
Member Author

rkalis commented Jan 18, 2024

Today we refactored the ConsoleParameterNode stuff so that it uses pre-existing compiler features for symbol table lookups. We also merged the ANTLR changes from master.

Next time we want to continue looking at the compiler stuff (most of the other GenerateTargetTraversal code). After that we want to refactor utils/source-map.ts and from there we can continue on to the libauth template generation stuff.

@rkalis
Copy link
Member Author

rkalis commented Jan 23, 2024

Today we finished refactoring the cashc and utils code. Next we will start refactoring the cashscript SDK code.

Looking at the CashScript SDK code, some private properties of classes have been made public. I remember when pat walked us through the code, that this was because that data is required when building the libauth template. We should see if we can change the interface of that function so that it requires fewer public properties. But this is not incredibly important if we can't change it.

We also noticed that there is a special case for the MockProvider in transaction.send(). Ideally this transaction.send() function would be fully provider-agnostic and the special case code should be moved into the MockProvider if possible. Perhaps we also need to look at changing the Libauth template builder interface to enable this.

We also looked at the error messages generated when a transaction failed (see https://twitter.com/CashScriptBCH/status/1745102982255939640), and we should try to make the structure of this message easier to parse quickly (such as changing Test.cash:5 Error to Error in Test.cash on line 5 and splitting up the message on multiple lines, etc).

Maybe we should also add a function to export the built libauth template (similar to bitauthUri() but without converting it to a bitauth URI).

At some point we should probably try to make the JestExtensions export a bit cleaner (e.g. cashscript/test instead of cashscript/dist/test), but that is not a high priority.

@rkalis
Copy link
Member Author

rkalis commented Feb 1, 2024

Today we added an additional test that checks the compiled source map with our fixture source map. We noticed that the source map generation fixtures were not the same as what was being generated by the latest compiler. We spent a bunch of time debugging, but we have a good indication that the current version is correct. Next time we just want to go back a few commits in the past again to double check that the source map fixture for TransferWithTimeout was definitely incorrect.

We also ran into a few other things:

  • It could be cool to incorporate all potential functions into a single Libauth Template (right now it is only for the function that is being called).
  • When using debug functionality (like debugTemplate, getBitauthUri etc), and the artifact does not contain debug information, we should disallow or at least warn the user of this.
  • If a source file has multi-line comments, then our bitauth script "sourcemap" breaks. We should probably solve this by escaping the end-comments (ie replace */ with *\/).

@rkalis
Copy link
Member Author

rkalis commented Feb 23, 2024

We double checked that the initial source map fixture was incorrect / outdated, so the one that we replaced it with is correct.

Today we also updated the console.log tests to include more edge cases. Here we noticed some weird behaviour on the JestExtensions file that we added a TODO for in the code. We can look into fixing that next time.

We also solved the issue of multiline-style comments breaking the BitAuth IDE integration due to prematurely ending the "sourcemap" comments.

Next time we want to also update the tests for require statement handling, and look at the JestExtensions stuff mentioned above. After that we want to add tests / fixtures for the full Libauth Template generation process. And the session after that we can finally get to refactoring the libauth template generation process.

@rkalis
Copy link
Member Author

rkalis commented Mar 1, 2024

Today we double checked the require statement tests, which were mostly fine. Then we got started on defining fixtures for Libauth Template generation (mostly so we can be more confident when refactoring). During this process we found some small issues with the template generation that we fixed.

There are still a few edge cases for which we want to add template generation fixtures, which we'll do next time + the aforementioned JestExtensions issues. After that we can get started on refactoring the libauth template generation.

@rkalis
Copy link
Member Author

rkalis commented Mar 8, 2024

Today we added the remaining edge case fixtures for template generation. We probably want to support other hashtypes and signing algorithms in the future, but we decided to skip that for now, since we couldn't think of a very good use case that really needs that.

After that, we implemented the fix that Pat sent us in telegram for an issue with if-else blocks. This turned out to affect some of the existing fixtures/tests, so we updated the fixtures, which were technically incorrect before.

While debugging, we encountered an issue where failed Libauth template evaluation in transaction.send() would report "Error [Object object]". So we looked into the debugTemplate function and noticed that it was throwing errors in different ways (some Error objects, some strings, and presumably either createProgram or vm.debug is also throwing error objects in certain cases). We need to make sure that all these errors are thrown in the same way and are forwarded to the user message correctly.

Next time we want to start the refactor, and the debugTemplate function looks like a good place to start so we can tackle the issues mentioned above.

PS: We're postponing dealing with the JestExtension stuff, since it's less important.

@mainnet-pat
Copy link
Contributor

Would you make a new @next release?

@rkalis
Copy link
Member Author

rkalis commented Mar 19, 2024

Today we published the new next release and shared it in the compilers group - after quickly fixing some fixtures that we forgot to update last time. After this we looked into Sayoshi Nakamario's issue that he reported to Mathieu. In the end we asked him to create a reproduction repo so we can build a test case from it. Finally we made a start on adding tests for the JestExtensions functionality where we ran into some weirdness that we'll tackle next time - we wrote down the TODOs in the code.

@rkalis
Copy link
Member Author

rkalis commented Mar 26, 2024

Today we looked at Sayoshi Nakamario's issue. It turns out that some of our fixes in 0.10.0-next.3 solved the issue.

After this we expanded our debugging tests and added more JestExtensions tests. Most of these are still failing. We think this is because of a bug in the debugTemplate() function, causing console.log statements to be executed in branches that were not executed.

Next time we will try to make the tests succeed by finding and fixing those bugs.

@rkalis
Copy link
Member Author

rkalis commented Apr 12, 2024

Today we fixed some bugs in the console.log statement handling. It looks like console.log statements are now fully working and have a good test coverage.

We also ran into new bugs with the require statement message handling for the final require statement case. For this we've created a list of new test cases we need to create. We've also implemented a likely fix, but we'll need to verify it with the new test cases once those are implemented.

There is also still one outstanding issue with the JestExtensions, where the JestExtensions will pass a test that expects a failure, even when no failure occurs.

We didn't have time to commit the require statement changes, so we want to do another session in the coming days to finish that and look at the final JestExtensions issue.

@rkalis
Copy link
Member Author

rkalis commented Apr 14, 2024

Today we fixed the last remaining issues with require statement handling. We also expanded the test cases so now we're pretty confident that most edge cases are fully covered. We also moved all debugging related functionality into a new debugging.ts file and refactored that code.

We also figured out the issue with the JestExtensions bug, but didn't have the time to fully implement a fix, so I'll do that early next week and then also release a new prerelease version.

@rkalis
Copy link
Member Author

rkalis commented Apr 15, 2024

I just did the last fixes to the JestExtensions and published a new next release. Next time we'll start refactoring the LibauthTemplate generation.

@mr-zwets
Copy link
Member

mr-zwets commented Apr 17, 2024

Noticed 2 issues while testing

provider.addUtxo(contract.address, randomUtxo() doesn't have the same behavior as
provider.addUtxo(contract.tokenAddress, randomUtxo()
so it seems there is a bug with the mocknetwork provider

When you create a single randomUtxo and use automatic input selection (without .from()), there are two inputs created in the libauthTemplate, which causes the contract to fail is certain situations (like tx.inputs.length or this.activeInputIndex)

edit: 2nd issue was on my end. However during testing I aslo noticed an issue with SigChecks not throwing the correct error

@rkalis
Copy link
Member Author

rkalis commented Apr 25, 2024

Today we fixed the first issue mentioned by @mr-zwets above - we now index on lockingBytecode rather than address.

Then we wrote tests for an issue that @mr-zwets had run into where signature checks that failed the NULLFAIL rule did not display custom require messages. We then fixed this issue (so now it still displays the require message when a signature check fails the NULLFAIL rule).

While we were working on that issue we noticed some issues with the template generation regarding signatures:

  1. The template generation always assumes that sig parameters get passed in as SignatureTemplates (but they can also be literal byte sequences).
  2. The template generation always assumes that sig parameters use Schnorr signatures. This causes issues with multisig checks which do not support Schnorr (at least not the way CashScript implements multisig).

We decided to tackle these issues while working on the Libauth template generation refactor, since we didn't want to touch just those parts. We made a start on the overall refactor and plan to continue next time (we documented our progress with TODO comments in the WIP code).

@rkalis
Copy link
Member Author

rkalis commented May 1, 2024

Today we refactored the generation of template.entities and template.scripts. Next time we still need to refactor the following parts of the libauth generation:

  • All P2PKH / signature template related things (e.g. p2pkh_placeholder_unlock (not including changes in functionality yet)
  • template.scenarios
    • createScenarioSourceOutputs
    • createScenarioTransaction

If we want to we can then tackle the other signature template related todos that are mentioned in the comment above. We also still want to tackle some other new functionality, most importantly, we still want to log the specific require statement (which line), even when users don't specify a custom error message.

There also might be some other TODOs sprinkled in the code that we should look at.

Before releasing the full new version, we also need to make some docs changes:

  • Document transaction.debug() and transaction.getLibauthTemplate()
  • Double check the require statement and console.log docs
  • Go over examples throughout the docs and update if needed
  • More docs mentioning MockNetworkProvider for testing purposes
  • Mention playground in the debugging docs

@rkalis
Copy link
Member Author

rkalis commented May 23, 2024

today we almost finished the refactoring, but we still need to push the final changes to createScenarioTransaction. While refactoring, we ran into this Libauth issue (bitauth/libauth#133), so we'll wait to hear back from Jason about that and push the final refactors after.

In the past weeks we also ran into some more issues that we want to fix before full release:

  • console.log causes bytecode changes in certain cases, due to it changing the OP_PICK / OP_ROLL behaviour of CashScript's stack management. This can be overcome, but we'll need some medium size changes to the way we handle logging
  • Double check zero-handling (should 0 be 0x or 0x00 and when) and add this to the automated tests
  • MockNetworkProvider does not work with anything timelock related because values are hardcoded
  • 'slot' -> What to do if a contract has different behaviour depending on input index, how do we evaluate all indexes

@mr-zwets
Copy link
Member

mr-zwets commented May 23, 2024

here's a proposed checklist

for 0.10.0-next.5

  • upstream libauth changes to allow for P2SH32 outputs in template
  • publish next release

for the full v0.10.0

  • solve issue failing test case require statements
  • fix timelocks on mocknet
  • add failing test console.logs should retain identical bytecode
  • rework console.log internals not to change bytecode
  • allow for final statement to be console.log
  • add two more tests fixtures (p2sh20 & tokens)
  • investigate and fix zero-handling
  • log the specific require statement (which line), even when users don't specify a custom error message
  • investigate slot workings
  • allow for ECDSA signing in template generation
  • move Jestextensions (for example to cashscript/test)
  • When using debug functionality (like debugTemplate, getBitauthUri etc), and the artifact does not contain debug information, we should disallow or at least warn the user of this.
  • Ideally this transaction.send() function would be fully provider-agnostic and the special case code should be moved into the MockProvider if possible
  • debugTemplate function and noticed that it was throwing errors in different ways (some Error objects, some strings) make sure that error is thrown to user, never [object object]
  • remove cashproof dependency

go over any and all TODOs left in the code

before releasing v0.10.0

  • Document transaction.debug() and transaction.getLibauthTemplate()
  • Double check the require statement and console.log docs
  • Go over examples throughout the docs and update if needed
  • More docs mentioning MockNetworkProvider for testing purposes
  • update NetworkProvider documentation
  • Mention playground in the debugging docs
  • reach out to have syntax highlighting plugin updated

@rkalis
Copy link
Member Author

rkalis commented May 28, 2024

We essentially finished all of the big refactoring. We encountered some issues with Libauth not fully supporting P2SH32 in the BCH_SPEC VM. We solved this by using the BCH_2023_05 VM instead. This looks to be working properly. We want to add two more tests for completeness: one that uses token introspection opcodes, and one that uses p2sh20.

We cut a new 0.10.0-next.5 release with all the refactors + some bug fixes. We updated the todo list above for the coming time.

Note about multiple input handling / advanced transaction builder:

Earlier we mentioned that we're not sure how multiple inputs can be tested at once, since there can only be one 'slot' field in a scenario. We'll have to discuss this with Jason, but our expectation is that we'd need to generate multiple scenarios, one for each input, and check that they all pass.

However, we noticed that our scenarios do support P2PKH inputs, which are evaluated separately. So it does seem like it's possible to evaluate multiple inputs using multiple scripts. We will need to look into this further.

@rkalis
Copy link
Member Author

rkalis commented Jun 4, 2024

Today we looked into a bug with require message handling that Mathieu found. This was caused by an incorrect instruction pointer due to removal of the final verify statement in the bytecode generation.

We tried to move the "final verify remove" code out of the main bytecode generation process and into the static optimisation process (so that the debugging tooling uses the code without the final VERIFY removed). However, this turned out to be very hard if not impossible, due to the fact that we need to remove the final VERIFY for every function, and we lose the function / AST context once we reach the static optimisation stage of compiling.

We've scheduled another session this week to fix the issue in a different way.

@rkalis
Copy link
Member Author

rkalis commented Jun 11, 2024

Today we finally solved the require message handling edge case that Mathieu found. We also tried to reproduce the mocknet timelock issue, but couldn't reproduce it. Next time we'll need to fix the console.log internals so that console.log statements don't impact production bytecode. We added a first failing test for this.

@rkalis
Copy link
Member Author

rkalis commented Jun 18, 2024

Today we fixed the two bugs with console.log statements.

One bug we fixed was that console.log statements could not occur as the final statement, even though they don't actually add any opcodes to the compiled bytecode. console.log statements can now be used as the final statement.

The other bug was that console.log statements would change the production bytecode due to stack handling. We changed the way these console.log statements are stored in the artifact so that this bug no longer occurs.

We also updated the tests so this functionality is properly tested.

We also removed cashproof as a devDependency and updated the developer docs on how to run cashproof.

We made a start towards logging specific require statements (line numbers) even when these statements don't use a custom message. We'll finish that work next time.

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

No branches or pull requests

3 participants