store and allow retrieval of transaction results

[psq]

This is work in progress, and by no means something to consider merging yet.

Background: as best I could tell, the way to get data about transactions without directly scanning the block database is to use Sidecar. You get a lot of information about each transaction, including all the events emitted (sweet!), but not what the transaction returned, which can be quite useful, and was lacking to implement a client for the Clarity hackaton.

Fortunately, the stacks-node server was emitting this data, so there was not much needed to be added to Sidecar. This PR includes the bare minimum I needed so far, but now I'd like to start a discussion on whether this should be something I should pursue further. See questions below.

With this PR, this lets you write client code like the one for the swapr contract where you get the (list dx dy) values returned by the transaction by just deserializing the content of tx_result which is ClarityValue. (calling this contract method)

Questions:

• is this the right approach and worth pursuing further (adding tests, implement the Postgres side of things), or were you envisioning something else?
• would you also store a more human readable version (like it is the case for events where both the hex and repr version are available)?
• there is a result field that is not being used (https://github.com/psq/stacks-blockchain-sidecar/blob/feature/raw_result/src/event-stream/core-node-message.ts#L119), what's the status of this, is this something someone is actively working on?

Bonus question: any doc or pointers on how to setup Postgres anywhere?

[aulneau]

Hi @psq!

I don't actively work on this project but I do work on other things that consume it directly (stacks-explorer), and first of all I wanted to thank you for this! I was just talking about these features and how they'd be useful to expose within the sidecar. I don't have much to add as far as the architectural aspects of this PR, but wanted to let you know that we'll be able to answer your questions soon.

would you also store a more human readable version (like it is the case for events where both the hex and repr version are available)?

I think it seems to be the case that we are moving toward returning most things with that kind of shape, eg:

{
  hex: ...,
  repr: 'something human readable',
}

I know that in the applications I work on, it's very helpful to have the sidecar handle and deserialization/conversion from clarity values.

For the other stuff, I feel like @zone117x should be able to help out!

[psq]

@aulneau adding a human readable deserialized field should be fairly easy, will add this while I wait to hear back

[psq]

hmm, what happened to all the ci tests? Or was it because the last commit was trivial stuff that it didn't trigger? I was hoping to get it to rerun the tests as some are failing locally (faucet, pg) to see if it was better than last time.

@aulneau I changed tx_result to return

{
  hex: '0x0100000000000000000000000000000001',
  repr: '1',
}

instead of returning just the hex value, which should be similar to the way it is done for other CV values.

[CLAassistant]

All committers have signed the CLA.

[zone117x]

Thanks for working on this @psq! Can you rebase your changes onto latest master? This PR should be good to go once the PR checks are passing, and a datastore unit test for the new field

[psq]

definitely. The rebase did some very strange things, so I've redone it locally but now I'm getting the same block from the mempool added twice the first time I run a transaction on the node, so will looking at this before pushing a clean set of commits.

[psq]

@zone117x I've rebuilt the PR, looks like it should have now.

Not sure what you mean by adding a datastore unit test, as I added support for the raw_result field in datastore-tests.ts to make it lint and pass, but perhaps you have something else in mind?

Also, I have no idea why the npm run build fails on the docket test, as this works fine locally. It looks like it is running on a different version of api-tests.ts as some line numbers do not match... And the test failures on the same file look off as well. Running npm test src/tests/api-tests.ts works fine locally. So any insight you could provide on how to make these tests pass on github would be welcome...

I figured out why the node was sending the tx twice: I was specifying the observer twice, once with an environment variable, and once from the toml file. That didn't use to cause issues, but that was before the mempool txs were added.

[zone117x]

Also, I have no idea why the npm run build fails on the docket test, as this works fine locally. It looks like it is running on a different version of api-tests.ts as some line numbers do not match... And the test failures on the same file look off as well. Running npm test src/tests/api-tests.ts works fine locally. So any insight you could provide on how to make these tests pass on github would be welcome

Not sure what's going on here either. It's working locally for me as well. Might be some kind of caching issue in the github actions, or maybe something wrong with how PRs from forks are handled. I'm going to pull this into another PR for debugging purposes

[zone117x]

Ahh I see the issue now. The PR just needs rebased on latest master (another PR was merged yesterday after your last rebase). Then the few test failures will show up locally and look like easy fixes

[zone117x]

@psq btw I pushed a small fix (f23f586) to your branch for the schema types. The docs/index.d.ts file had manual edits, but this file is auto-generated from contents of docs/**/*.schema.json

[psq]

Ahh I see the issue now. The PR just needs rebased on latest master (another PR was merged yesterday after your last rebase). Then the few test failures will show up locally and look like easy fixes

Ah, yes, very good point, it runs the tests after merging with master, not the code in the PR branch. Fixing now.

@psq btw I pushed a small fix (f23f586) to your branch for the schema types. The docs/index.d.ts file had manual edits, but this file is auto-generated from contents of docs/**/*.schema.json

makes sense, thank you.

[codecov]

Codecov Report

Merging #129 into master will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage   56.87%   57.02%   +0.15%     
==========================================
  Files          40       41       +1     
  Lines        2553     2562       +9     
  Branches      404      406       +2     
==========================================
+ Hits         1452     1461       +9     
  Misses       1098     1098              
  Partials        3        3              

Impacted Files	Coverage Δ
src/datastore/common.ts	67.05% <ø> (ø)
src/event-stream/core-node-message.ts	0.00% <ø> (ø)
src/api/controllers/db-controller.ts	59.69% <100.00%> (+0.62%)	⬆️
src/datastore/postgres-store.ts	83.37% <100.00%> (+0.03%)	⬆️
src/migrations/1593415759720_add-txs-raw-result.ts	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 762ea51...8b7d3a7. Read the comment docs.

[psq]

@zone117x yes, that was it. tests are back to green now 😂

[zone117x]

Looks great, thank you @psq!

[psq]

great, thank you for your help on this @zone117x
I don't have write access to the repo, so someone will have to merge it, unless someone else needs to review?