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

RPC data and its format #18

Closed
randomlogin opened this issue Sep 27, 2024 · 5 comments · Fixed by #25
Closed

RPC data and its format #18

randomlogin opened this issue Sep 27, 2024 · 5 comments · Fixed by #25

Comments

@randomlogin
Copy link
Contributor

randomlogin commented Sep 27, 2024

I'd like to propose a change in the RPC response format to make it more coherent.

General:

  1. Omit empty fields if they are not required.
  2. Decide about required and optional fields.
  3. If the needed object is not present/found, return an error.
  4. Stick to bitcoind field names where possible.

Blocks

getblockdata

Current response:

{
    "jsonrpc": "2.0",
    "result": {
        "tx_data": [...]
    "id":1
}

Return an error for a missing block

Right now it returns the same response for a non-existing block and for a block without spaces txs:

Block #40435 (00000000b2a106a004d09c71d4f1079f349eda1e50c68a2563ab9bbcb67197e1)

{
    "jsonrpc": "2.0",
    "result": null,
    "id": 1
}

Non-existing block (ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff) has the same response.

Expected response for a non-existing block:

{
    "result": null,
    "error": {
        "code": -5,
        "message": "Block not found"
    },
    "id": 1
}

Error codes for bitcond.

Include block header information

Bitcoind header:

{
        "hash": "000000000000004c1d5bbc5b3f6693f25b045ba66434fd6ba46c792177f2d63e",
        "confirmations": 7015,
        "height": 40345,
        "version": 545259520,
        "versionHex": "20800000",
        "merkleroot": "84d91c376fb7406009043374dc3ac6fb27e2d7acf650f1b485680da3618cb641",
        "time": 1724098163,
        "mediantime": 1724094633,
        "nonce": 766356620,
        "bits": "194d9574",
        "difficulty": 55358228.00276604,
        "chainwork": "000000000000000000000000000000000000000000000065d0ab1832185c8f4b",
        "nTx": 18,
        "previousblockhash": "0000000004a3c906e6795f2d362521783412ee70a37fff840f6909d05cdbf2da",
        "nextblockhash": "0000000000000035ae65009fbd6bc1a8e1f46b24407267bb75da1da43ae8231a",
        "strippedsize": 3882,
        "size": 7087,
        "weight": 18733,
        "tx": [...]
}

I'd say that the following fields are required:

  • height,
  • hash block hash,
  • previousblockHash,
  • nextblockHash (explicit null if there no next block hash),
  • tx (empty erray if there are no spaces-related txs in the block)

Other fields should be optional, omitted if not applicable/known.

Renaming

  • tx_data -> tx

Transactions

Add gettransactiondata

If there is tx indexing, create an RPC call which returns spaces-related information from a transaction.
Compare: bitcoin getrawtransaction.
RPC: gettransactiondata (consider gettransaction or getrawtransaction and also renaming getblockdata into getblock)
Params:

  • txid
  • verbosity level?

Response format:

  • txid required
  • blockhash required
  • locktime (is it needed?)
  • vin required,
  • vout required,
  • vmetaout required, empty array if there are no spaces-related actions in this tx

All other fields of should be considered optional.

Renaming

  • lock_time -> locktime

Vin

Current format:

{
   "previous_output":"46f90824ab1556eb5312cbe456a51ead6f283d3938fcfd6e42e0421bfc6e0875:2",
   "script_sig":"",
   "sequence":4294967293,
   "witness":[
      "6eb3bb1075211f1aee8fb0dd1750ed1694d5b2b1f1cc3626a77c9b7b6dbc79a41957691f89857304826d9f216704bedbfec3acc8a65d33b09fb6ba36a6e2d0c2",
      "13dededede0e000b0961727466756c64657600017520dd2ef20b900ef6bd4dbca87170edcfa3f20396fe912cb2f010504f521409cf4dac",
      "c1dd2ef20b900ef6bd4dbca87170edcfa3f20396fe912cb2f010504f521409cf4d"
   ],
   "script_error":null
}

Bitcoind format (non-coinbase input):

{
   "txid":"64c5c91bf3a050cbae12d5162a6310bf6511677c80da11892d0a5d88ef03c3c9",
   "vout":1,
   "scriptSig":{
      "asm":"",
      "hex":""
   },
   "txinwitness":[
      "031b6b67cc82a50784989ca297978d6180171b8a8c8596b21cb6d65c593f35db1746bd46b21689b659a29822643be388c1b906058f1784b887803e408f73fc82"
   ],
   "sequence":4294967293
}

Bitcoin format (coinbase input):

{
   "coinbase":"03999d00000473a6c36604478e95211076e1b56600000000910e0100000000000a636b706f6f6c22506f72746c616e642e484f444c207c20446574726f6974207c20f09f87baf09f87b8",
   "txinwitness":[
      "0000000000000000000000000000000000000000000000000000000000000000"
   ],
   "sequence":4294967295
}

Additional fields

Split outpoint into two fields:

  • txid
  • vout

I have some concerns about the name txid of a field for vin, as it might be confused with the txid of the tx where they appear.

Renaming

  • script_sig -> scriptSig
  • witness -> txinwitness

Thus resulting in the following fields:

  • txid (optional, omitted if coinbase)
  • vout (optional, omitted if coinbase)
  • coinbase (optional, present if coinbase)
  • scriptSig (optional, hexstring)
  • txinwitness required
  • sequence required
  • script_error - what is it? should be omitted if empty

Vout

Current format:

{
   "value":1001,
   "script_pubkey":"6a4101bb192f35066882cb907198b8dddae89bb7705c155c25b82454fa49968c99f59a5931c3df4c220c12ddb3a6f07b6ff27fcf6a0176001207a819e79661735f3df2"
}

Bitcoind format:

{
   "value":0.00000662,
   "n":2,
   "scriptPubKey":{
      "asm":"1 47d39e3f59786a603c87694c68b9125568a7d59002b115b373773061d58a12a6",
      "desc":"rawtr(47d39e3f59786a603c87694c68b9125568a7d59002b115b373773061d58a12a6)#aqnujjqv",
      "hex":"512047d39e3f59786a603c87694c68b9125568a7d59002b115b373773061d58a12a6",
      "address":"tb1pglfeu06e0p4xq0y8d9xx3wgj245204vsq2c3tvmnwucxr4v2z2nq4jy0vv",
      "type":"witness_v1_taproot"
   }
}

I think it's okay to leave value nominated in satoshis and return only hex of scriptPubKey.

Additional fields:

  • n index of output

Renaming

  • script_pubkey -> scriptPubKey

Vmetaout

Current format.

{
   "outpoint":"a9226ee6316b88737776fab48824f9694c184bcc75577c7844bec2363613a820:1",
   "value":662,
   "script_pubkey":"5120396909d29e4d68464d51b7009be7e2ca26eb5d8122e1f3b32a668b11f031144a",
   "name":"@bitcoin",
   "covenant":{
      "type":"bid",
      "burn_increment":223,
      "signature":"aa6e6b5ecf6085cdbfffd0c7907418d4f5a0da532eaa8309d0e0831afb02f47d19edbb8ee30711e7a54f062ce9238bf9f56500cdc31b9561900ba5f2b29d2a28",
      "total_burned":3000,
      "claim_height":40489
   }
}

Current format of a 'bad' action:

{
   "action":"reject",
   "name":"@artfuldev",
   "reason":"AlreadyExists"
}

I think 'bad' action should still show the required fields and also add an error. The field action will be redundant in the presence of an error field.

Show outpoint txid and index as distinct fields, not colon-separated.

Current format:

...
 "outpoint": "a9226ee6316b88737776fab48824f9694c184bcc75577c7844bec2363613a820:1",
...

Proposed format:

...
"outpoint_txid": "a9226ee6316b88737776fab48824f9694c184bcc75577c7844bec2363613a820",
"outpoint_index":1,
...

Also the fields can have titles txid and vout, but as they do not exist in usual bitcoin, we can be more explicit.

Renaming

  • script_pubkey -> scriptPubKey

Resulting fields

  • outpoint_txid required, hexstring
  • outpoint_index required
  • name required, string
  • value required

Covenant:

  • type required, has three possible variants: "bid", "transfer", "reserve"
  • claim_height optional, omitted if empty
  • expiry_height optional, omitted if empty
  • signature required
  • error optional, string, required if the action was not successfull
@buffrr
Copy link
Member

buffrr commented Sep 27, 2024

Return an error for a missing block

Agreed

Include block header information

It would add 80 bytes extra storage to the index per block but it already exists in Bitcoin core. Could you clarify the utility of this?

Add gettransactiondata

We could add this as a helper method. However, it would do basically query Bitcoin RPC to figure out which block the transaction is in (assuming txindex is enabled in bitcoind) and then query our block index to get the transaction.

Alternative approach

I wonder if we're getting ourselves into a deep rabbit hole here trying to match Bitcoin Core responses. Some types are defined by the bitcoin crate and it may not match bitcoind exactly so we might have to write custom serializers for those too.

What if we split the transaction object itself from the protocol specific annotations? Something like this:

{
    "tx": <bitcoin-tx-exact-format-as-bitcoin-core>,
    // all protocol specific annotations would be here
    "meta": {
          "vin": [ {
              "n": 2, // input index in the referenced tx,
              // any metadata for that input e.g. is it a space spend ... etc
          }, 
         ...  
         ],
         "vout": [ {
              "n": 1, // vout index in the tx
              "txid": null, // null or omitted if part of the transaction itself otherwise for bid outputs this would include the external txid,
              // any meta information like space covenants or errors
             }
          ]
    }
}

This structure would separate the core transaction data from our protocol-specific metadata. The "tx" field would maintain exact Bitcoin Core format, while the "meta" field allows us to add any necessary annotations without affecting the core structure.

You also mentioned the desire to have everything in vmetaout, and this structure would enable that. What are your thoughts? If you like this, could you see if the above structure actually fits our model well? It should very flexible we could even move the revokes to the vins instead of vmetaout like previously which makes more sense.

Sidenote: If we go with this perhaps we want to avoid providing the tx at all and just have a method called gettransactionmeta and getblockmeta or something just to get the meta information. It would also reduce storage needed for the index since all tx data is already in Bitcoin core.

@buffrr
Copy link
Member

buffrr commented Sep 27, 2024

Another suggestion for the meta information would be a more operation oriented structure something like this:

{
    // All outputs spent/removed from our spaces utxo set 
    // e.g. ones the protocol no longer wishes to track
    "spends": [
     {
       "n": 1, // input index in the tx,
        // other useful info
     },
    ],
    // all outputs created these are just outputs in the tx
    "creates": [ 
      {
        "n": 2, // output index associated with this space
        "space": "@bitcoin",
        "covenant": {...} 
     }],
     // mainly updates to states of outputs nominated for auction in this tx go here
    "updates": [
      {
        "txid": "<txid>",
        "n": 1,
        "space": "@money",
        // ...   
    }] 
}

@randomlogin
Copy link
Contributor Author

It would add 80 bytes extra storage to the index per block but it already exists in Bitcoin core. Could you clarify the utility of this?

Initially I thought about ensuring that spaced has the same blocks as those of bitcoin, to have at least prevBlockHash there. But it can be done through other ways. So yes, header seems useless.
What do you think about leaving blockhash in the response?

I wonder if we're getting ourselves into a deep rabbit hole here trying to match Bitcoin Core responses. Some types are defined by the bitcoin crate and it may not match bitcoind exactly so we might have to write custom serializers for those too.

Got it. Yes, if bitcoin crate has different type names, it's might be cumbersome to write custom serializers.

Then I guess we can not follow bitcoind format.

Another suggestion for the meta information would be a more operation oriented structure something like this:

Looks fruitful, at least for block explorer purposes, but I'm not yet sure whether it's okay for other use cases (which ones?). It feels like it forces state tracking, not just returning the data.

@buffrr
Copy link
Member

buffrr commented Sep 27, 2024

What do you think about leaving blockhash in the response?

The existing RPC method getblockdata(blockhash) already expects a blockhash by the caller so it would be redundant. There's no method for retrieving by block height (we don't keep an index for that)

Looks fruitful, at least for block explorer purposes, but I'm not yet sure whether it's okay for other use cases (which ones?). It feels like it forces state tracking, not just returning the data.

Internally, the protocol crate processes txs this way:

  1. prepare method which takes a bitcoin transaction, loads all data necessary for validation and returns a PreparedTransaction object.
  2. validate: The validator is completely decoupled from io/network logic so it takes a PreparedTransaction and turns it into a ValidatedTransaction which would be updated to use the proposed structure.

The node crate is quite simple it reads validated transactions and applies state changes to the tree. The proposed structure is flexible enough for that use case and seems flexible enough for the explorer too.

However, it is more low level and raw compared to simpler APIs but it's comprehensive so it has all the details that could possibly be needed. IMO, It's better to leave higher-level, user-friendly APIs to dedicated indexers like the one you're building to ensure the core code is lean, have minimal dependencies and focused on consensus.

@randomlogin
Copy link
Contributor Author

Another suggestion for the meta information would be a more operation oriented structure something like this:

{
    // All outputs spent/removed from our spaces utxo set 
    // e.g. ones the protocol no longer wishes to track
    "spends": [
     {
       "n": 1, // input index in the tx,
        // other useful info
     },
    ],
    // all outputs created these are just outputs in the tx
    "creates": [ 
      {
        "n": 2, // output index associated with this space
        "space": "@bitcoin",
        "covenant": {...} 
     }],
     // mainly updates to states of outputs nominated for auction in this tx go here
    "updates": [
      {
        "txid": "<txid>",
        "n": 1,
        "space": "@money",
        // ...   
    }] 
}

It seems it's the most useful format. Can we stick to this?
Also I think we should list erroneous attempts that didn't result in any state change (if it violates protocol, does it still spend old create new output?).
Maybe we can add a new top-level field "errors" among "creates", "spends", "updates".

What are possible fields for the covenant?

randomlogin added a commit to randomlogin/spaced that referenced this issue Oct 22, 2024
buffrr added a commit to buffrr/spaces that referenced this issue Nov 5, 2024
@buffrr buffrr mentioned this issue Nov 6, 2024
@buffrr buffrr linked a pull request Nov 6, 2024 that will close this issue
@buffrr buffrr closed this as completed in #25 Nov 7, 2024
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

Successfully merging a pull request may close this issue.

2 participants