hirosystems / clarinet

Write, test and deploy high-quality smart contracts to the Stacks blockchain and Bitcoin.
https://hiro.so/clarinet
GNU General Public License v3.0
303 stars 138 forks source link

Add support to retrieve the observable events from the contracts in a given Clarinet project #1034

Closed sabbyanandan closed 7 months ago

sabbyanandan commented 1 year ago

As a user, I want the ability to discover all observable chainhook events in a given project so I can look at the list of events and create the chainhooks easily as opposed to manually defining each predicate by reading the docs, making errors in the process, and wasting so much time.

The rest of additional context is at https://github.com/hirosystems/platform/issues/546, and I am skipping it for brevity here.

Previous context: ~We want to use clarity-events in other places, so that requires making this component a JavaScript-readied WASM library.~

lgalabru commented 1 year ago

We probably want to see a bunch of our libraries being exported to wasm. @hugocaillard what's your take on introducing a component clarinet-sdk, that would essentially be some sort of umbrella component, along with exposing a few wasm entrypoints. This SDK could be published both on crates.io and npm. The platform would consume the wasm version available on npm (for the clarity-events, the deployment plans, and maybe other future features like formatter, etc). I guess the Vs code extension could potentially also become a consumer?

hugocaillard commented 1 year ago

Totally makes sense. Non exhaustive list of WASM components that could be expose

With this approach, the clarinet-sdk would be the only one requiring wasm-bindgen

Although I'm not sure of the need to publish anything on crates.io? What's the use case?

I guess the Vs code extension could potentially also become a consumer?

Yes

andresgalante commented 1 year ago

This work will be a dependency for https://github.com/hirosystems/platform/issues/546

hugocaillard commented 1 year ago

Sounds like we can tackle this in Q3 (September) with the progress made on the JS clarinet-sdk πŸ™Œ

hugocaillard commented 1 year ago

Hey @sabbyanandan I wanted to point out that the clarinet-sdk is mainly used to be used within Node.js, but not in the browser. So as it is, it could be used on the back-end but not on the front-end. We could do so, but it would also mean for the platform to provide a way for the SDK to "read" the files (contracts and Clarinet.toml). Within Node, it relies on Node FS, but in the browser, it could rely on a virtual file system (provided by the platform). What was the plan with this package?

@lgalabru also mentioned to possibility to get the events from the devnet package (I think it already returns the ABI, it could also have the events)

lgalabru commented 1 year ago

The ABI is unfortunately not part of the packaged plans, but this is something we can address, and would make sense to me, so I'm going re-purpose this issue. @csgui do you think you could help us with that?

hugocaillard commented 1 year ago

Also, I'm still interesting in adding it in the JS SDK, but just wanted to warn that it won't be available in the browser (at least not out of the box)

csgui commented 1 year ago

@csgui do you think you could help us with that?

Sure thing! πŸ‘

smcclellan commented 1 year ago

Idea: New command clarinet deployments package - @beguene to review technical implementation spec that @csgui writes up here.

csgui commented 1 year ago

After working on an initial implementation we have the following proposal to the specification:

Leveraging the use of the Clarinet contracts command, we can add a sub-command abi. The output can be a JSON format with both event and function types. Thus, the JSON output can be handled by the client, i.e.: filtering by type value.

Any feedback on that? @sabbyanandan @beguene @hugocaillard @lgalabru @MicaiahReid

(print "hello!")

(define-public (say-hello) 
    (ok u1))

;; ---------------------------

(define-private (transfer-to-recipient! (recipient principal) (amount uint))
  (if (is-eq (mod amount u10) u0)
      (stx-transfer? amount tx-sender recipient)
      (err u400)))
$ clarinet contracts abi contract-file.clar
[
  {
    "trigger": "say-hello",
    "name": "print",
    "data_type": {
      "value": "hello!",
      "type": "string"
    },
    "type": "event"
  },
  {
    "trigger": "transfer-to-recipient!",
    "name": "transfer_stx_event",
    "type": "event"
  },
  {
    "name": "transfer-to-recipient!",
    "access": "private",
    "args": [
      {
        "type": "principal",
        "name": "recipient"
      },
      {
        "type": "uint",
        "name": "amount"
      }
    ],
    "outputs": [
      {
        "type": "response",
        "value": {
          "ok": "bool",
          "err": "string"
        }
      }
    ],
    "type": "function"
  },
  {
    "name": "say-hello",
    "access": "public",
    "args": [],
    "outputs": [
      {
        "type": "string"
      }
    ],
    "type": "function"
  }
]
hugocaillard commented 1 year ago

Some thoughts about the data structure.

What about splitting the ABI and events like so:

{ events: [], functions: [] }

Or even having two commands

clarinet contract interfaces <contract-name>
clarinet contract events <contract-name>

(<contract-name> could be optional and return the data for all contract when omitted)

I think it also depends on the platform needs, @sabbyanandan

@csgui

andresgalante commented 1 year ago

@beguene @sabbyanandan would you mind reviewing the json output and adding feedback on this issue, please?

sabbyanandan commented 1 year ago

Hey @sabbyanandan I wanted to point out that the clarinet-sdk is mainly used to be used within Node.js, but not in the browser. So as it is, it could be used on the back-end but not on the front-end. We could do so, but it would also mean for the platform to provide a way for the SDK to "read" the files (contracts and Clarinet.toml). Within Node, it relies on Node FS, but in the browser, it could rely on a virtual file system (provided by the platform). What was the plan with this package?

@hugocaillard: My apologies for the delayed response. The DevEx and the use-case requirements for why clarity-events is useful (in the Platform) is explained at https://github.com/hirosystems/platform/issues/546 β€”Β it doesn't have to be a WASM module; as far as we have a lib, we can use that in the Platform backend to hopefully extract the event-streams for a given set of contracts in the project. And that's all we need.

sabbyanandan commented 1 year ago

Any feedback on that? @sabbyanandan @beguene @hugocaillard @lgalabru @MicaiahReid

@csgui: If I understand the data structure correctly, it appears we have a few different ways to get the "possible" event-streams for a given contract. (1) using the clarity-events package/lib directly in code (2) using the clarinet contracts abi or clarinet contracts events commands in Clarinet (3) using the RESTful API backing the Clarinet commands in (2)

Did I get this right?

Regardless of these options (it looks like we can get the data multiple ways, anyway), the goal for the Platform is to discover the possible event streams automatically and offer an improved DevEx for developers to select and create Chainhooks rapidly instead of manually drafting each Chainhook. I hope this clarifies the ask.

For further context into Chainhook improvements in the Platform, see here.

beguene commented 1 year ago

@csgui I think the output format you described makes sense, it's perfectly fine to have everything in a plain array. We can filter in our app if required. Agree with @sabbyanandan: as such we don't necessarily need a wasm as we have a node backend and a pod for each user with clarinet installed. Though having a clarity-events nodejs package will be easier and more performant for the platform.

hugocaillard commented 1 year ago

@beguene

Though having a clarity-events nodejs package will be easier and more performant for the platform.

So it looks like the first idea of having it on the clarinet-sdk still make sense.

Would something like that work for you:

import { initVM } from "@hirosystems/clarinet-sdk";

const vm = await initVM();

const contractInterfaces = vm.getContractsInterfaces()
// contractInterfaces interfaces is a map of all contract interfaces

const counterInterface = contractInterfaces.get(`${deployerAddr}.counter`);
console.log(counterInterface?.functions) // array of the functions

// not implemented yet / tbc
const events = vm.getContractEvent(`${deployerAddr}.counter`);

The clarinet-sdk is basically Clarinet wrapped in WASM. Also right now the package only work in Node.js (not in the browser), but that's something we could work on

beguene commented 1 year ago

@hugocaillard Yes this should work. Is initVM compute or memory intensive ? Looks like it's expecting a Clarinet.toml. Does this mean we also need all the contracts files in a well structured project like this ? If that's the case, we can't really do that in our backend but we can do it on the user's pod where the project folder and Clarinet.toml lie.

csgui commented 1 year ago

Does this mean we also need all the contracts files in a well structured...

Would be better to follow a pattern, like contract files under a contracts folder. And the contracts folder in the same place where the Manifest file (Clarinet.toml) lives.

csgui commented 1 year ago

@hugocaillard Thinking about the platform requirements seems that makes more sense to implement this on the clarinet-sdk.

hugocaillard commented 1 year ago

Long story short, the manifest file (Clarinet.toml) allows to get the exact path of each contracts along with the setting (clarity_version, epoch) that could change the result of the ABI. Also, because contracts call other contract (from the same project or from requirements from mainnet), it's necessary to load the whole session to know the ABI of a contract.

So the Clarinet.toml is a source of truth and a good way to make that our tools behave the same way.

Is initVM compute or memory intensive? - @beguene

It is slightly, it also depends on the size of the project. But it basically what happens every time the Clarity VSCode extension starts.

Looks like it's expecting a Clarinet.toml.

Yes

Does this mean we also need all the contracts files in a well structured project like this? - @beguene

Not necessarily, because the contracts path are specified in the manifest (Clarinet.toml), allowing for flexibility. (sbtc is using multiple directories)

[contracts.hello-world-v1]
path = "contract-v1-directory/hello-world.clar" # dir 1
clarity_version = 1
epoch = 2.0

[contracts.hello-world-v2]
path = "contract-v2-directory/hello-world.clar" # dir 2
clarity_version = 2
epoch = 2.4

[contracts.hello-world-tests]
path = "some-other-directory/hello-world.clar" # dir 3
clarity_version = 2
epoch = 2.4

Would be better to follow a pattern, like contract files under a contracts folder. - @csgui

I'm not sure if we want to add this constraint to Clarity developers on the platform, having the Clarinet.toml as a source of truth to list contract and get their paths and settings (epoch / clarity_version / maybe more in the future) is quite useful.


Ideas for the future

hugocaillard commented 1 year ago

@csgui @beguene

Thinking about the platform requirements seems that makes more sense to implement this on the clarinet-sdk.

Yes it does πŸ‘

So the ABI is already available if you want to start implementing that @beguene

const contractInterfaces = vm.getContractsInterfaces()

I can also prioritize implement contract events, it shouldn't be too long. What's your timeline @beguene @sabbyanandan?

(I could implement it this week it could make it the v1 or v1.1)

beguene commented 1 year ago

Got it @hugocaillard ! Sounds good to me. We anyway rely a lot on the Clarinet.toml for deployment plan and devnet so we can continue like this. Regarding the folder, I meant we must have a file system to use const vm = await initVM();. Hence we won't do it in the platform backend but on the user vscode pod where all the user's projects and contracts are persisted (just like we do now for deployment plan and devnet) cc @BLuEScioN

hugocaillard commented 1 year ago

@beguene Ok I see.

In the vscode pod, do you have access to the actual FS (ie: node:fs will work out of the box) or is it the VSCode virtual file system?

beguene commented 1 year ago

@hugocaillard We do have the actual filesystem so everything should work fine. We access it via a custom nodejs server and indeed we use node:fs to interact with the user's clarity project files.

hugocaillard commented 1 year ago

@beguene ok perfect, thanks. So two questions on my end:

Thx

sabbyanandan commented 1 year ago

@hugocaillard: Nakamoto/sBTC is the immediate priority, and access to events via the SDK can take a back seat until you free up. There is no rush; the Platform can pick it up whenever it is ready (even in a milestone capacity versus a full-blown release).

lgalabru commented 1 year ago

We anyway rely a lot on the Clarinet.toml for deployment plan and devnet so we can continue like this.

@beguene this makes me think that the Platform team could be missing a feature that we shipped in v1.8: packaged deployment plans. Thanks to this new command, you should be able to dump the entire project (including contracts, wallets) in one json file that you can use for executing the deployment plan - which should probably help you delete a lot of code on your end.

beguene commented 1 year ago

Nice! This is good to know. Yes, we missed it because we started working on deployment plan before this was available, but we use the devnet package command for our devnet feature, which is indeed cleaner. We will refactor this when we touch the deployment plan again. Those features in the clarinet are precious and make it easy for us to integrate into the platform.

hugocaillard commented 1 year ago

Will try to do it in October in sdk v1.1. Lower the the priority to medium

sabbyanandan commented 9 months ago

@smcclellan: Once 2.2 ships, could we timebox to review the scope and estimate this effort?

There are additional signals from devs using Chainhooks, but they also create things manually, which we can entirely automate once we automatically provide visibility into observable events.

smcclellan commented 9 months ago

@sabbyanandan Yes, this is on my radar. Thanks for flagging.

smcclellan commented 8 months ago

@hugocaillard to clarify the approach, possibly opening a new ticket to track the new scope.

hugocaillard commented 8 months ago

@sabbyanandan Considering all of the noise that happened on this issue, could we open a new one instead?

Ideally, I'd like the new issue to focus on the "what" / "why" instead of the "how". (eg, instead of "Include ABI + and Clarity events analysis in packaged deployment plans", we could have "platform need access to ABI and events analysis for ...")

sabbyanandan commented 7 months ago

@hugocaillard: I would repurpose this issue; on that note, I updated the title and description. I would appreciate it if we could scope this first and then decide on the timeline feasibility. This can be impactful for the Platform users.

hugocaillard commented 7 months ago

Thanks @sabbyanandan Have you considered opening a new issue as I suggested in my previous comment? I'll provide some details later today

smcclellan commented 7 months ago

Closing in favor of https://github.com/hirosystems/clarinet/issues/1410 to simplify the thread. /cc @sabbyanandan