cculianu / Fulcrum

A fast & nimble SPV Server for BCH, BTC, and LTC
Other
347 stars 78 forks source link

blockchain.transaction.relayable / status? #44

Open monsterbitar opened 4 years ago

monsterbitar commented 4 years ago

I had a talk with shammah about how he uses electrum for stamp, and we identified a feature that might be good to have: checking if a transaction is relayable or not.

For the case not, it could be due to it being a double-spend, or not matching standardness, or too low fee etc.

The call would take a hex-encoded raw transaction, or an array of such, and then return information about the transactions validity / relayability. It would be particulary useful in places that works with off-line transactions where part of the check for validity is that it's unspent and valid according to the network itself.

Input: Array of raw transactions, and optional broadcast flag. Output: Boolean for total outcome, array of txid:bool, and optional broadcast txid.

It might be possible to include this functionality as part of https://github.com/cculianu/Fulcrum/issues/32 (both as a standalone blockchain.transaction.status and as a subscription), rather than making a separate feature.

cculianu commented 4 years ago

Ok a few notes and questions:

So this would presumably use testmempoolaccept?

I'm not sure if the array-of-tx's thing is something I'm keen on. Fulcrum has a policy of not supporting compound calls such as this. Fulcrum doesn't even do JSON-RPC batching for that reason. There is a strong preference in the way the design of the server is to only allow for small, fast, light calls. I would say that is would be preferable to avoid passing array's of tx's but rather perhaps just do this as 1-tx-per-call.

In addition the array-of-tx's thing may return non-useful results. In pathological cases you can have an array of double-spends or conflicting tx's, and the way testmempoolaccept works at least on ABC and BCHN is that it only accepts 1 tx at a time (!! this despite the API accepting an array -- if you pass more than 1 it just errors out!!).

So anyway fulcrum would have to loop over the array, calling bitcoind each time .. and they will all return true here .. but in practice the set of tx's specified cannot be relayed together... so I am not sure how useful passing an array actually is.

I think if we did this as a 1-tx-per-call -- it would not only make Fulcrum's internals happy, but just be clearer as to what the "contract" is for calling code...

monsterbitar commented 4 years ago

I hadn't considered the compliactions of transactions in an array interacting with regards to their relayability.

I'm fine with dropping the array part of this, and do it on a 1-for-1 basis.

cculianu commented 4 years ago

Maybe we should open an issue in the protocol spec github?

Note that testmempoolaccept returns the following information:

 {
  "txid"           (string) The transaction hash in hex
  "allowed"        (boolean) If the mempool allows this tx to be inserted
  "reject-reason"  (string) Rejection string (only present when 'allowed' is false)
 }

The RPC call for Electrum Cash can return similar or verbatim that information...

EchterAgo commented 4 years ago

BU doesn't have testmempoolaccept but validaterawtransaction provides similar information:

{
  "txid" : "value",           (string) The transaction hash
  "isValid" : true|false,   (boolean) If the transaction has a complete set of signatures
  "isMineable" : true|false,   (boolean) If the transaction is mineable now
  "isFutureMineable" : true|false,   (boolean) If the transaction is mineable in the future
  "isStandard" : true|false,   (boolean) If the transaction is standard
  "metadata" : {
       "size" : value,        (numeric) The size of the transaction in bytes
       "fee" : value,         (numeric) The amount of fee included in the transaction in satoshi
       "feeneeded" : value,   (numeric) The amount of fee needed for the transactio in satoshi
    },  "errors" : [                 (json array) Script verification errors (if there are any)
      "reason",           (string) A reason the tx would be rejected by the mempool
        ...
    ],
dagurval commented 4 years ago

The validaterawtransaction is very detailed, the full API is response is here. If it's a superset of testmempoolaccept, then we could at least support what they have in common.

monsterbitar commented 4 years ago

From what I can see, it's unclear if "allowed" (boolean) has a direct mapping. Perhaps it matches to isFutureMineable?

cculianu commented 4 years ago

I would guess "allowed" in BCHN/ABC maps to "isMineable" (or perhaps "IsMineable" && "isStandard"). I think to be sure one would have to check the sources of BU. I can do that (since I'm already familiar with how BCHN does it, so I can compare) -- if someone else doesn't do it first...

EchterAgo commented 4 years ago

The equivalent of BCHN's allowed is BU's isValid.

monsterbitar commented 4 years ago

thanks @EchterAgo, I double-checked with BU and they will be updating the validaterawtransaction description to make it clear that isValid verifies more than just the signatures. They further clarified that you can receive multiple reasons when a transaction is not valid.