Open randomlogin opened 6 days ago
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.
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.
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 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.
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:
prepare
method which takes a bitcoin transaction, loads all data necessary for validation and returns a PreparedTransaction
object. 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.
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?
I'd like to propose a change in the RPC response format to make it more coherent.
General:
Blocks
getblockdata
Current response:
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
)Non-existing block (
ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
) has the same response.Expected response for a non-existing block:
Error codes for bitcond.
Include block header information
Bitcoind header:
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
(considergettransaction
orgetrawtransaction
and also renaminggetblockdata
intogetblock
) Params:Response format:
txid
requiredblockhash
requiredlocktime
(is it needed?)vin
required,vout
required,vmetaout
required, empty array if there are no spaces-related actions in this txAll other fields of should be considered optional.
Renaming
lock_time
->locktime
Vin
Current format:
Bitcoind format (non-coinbase input):
Bitcoin format (coinbase input):
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
requiredsequence
requiredscript_error
- what is it? should be omitted if emptyVout
Current format:
Bitcoind format:
I think it's okay to leave value nominated in satoshis and return only hex of scriptPubKey.
Additional fields:
n
index of outputRenaming
script_pubkey
->scriptPubKey
Vmetaout
Current format.
Current format of a 'bad' action:
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:
Proposed format:
Also the fields can have titles
txid
andvout
, but as they do not exist in usual bitcoin, we can be more explicit.Renaming
script_pubkey
->scriptPubKey
Resulting fields
outpoint_txid
required, hexstringoutpoint_index
requiredname
required, stringvalue
requiredCovenant:
type
required, has three possible variants: "bid", "transfer", "reserve"claim_height
optional, omitted if emptyexpiry_height
optional, omitted if emptysignature
requirederror
optional, string, required if the action was not successfull