search

lbryio / lbrycrd

The blockchain that provides the digital content namespace for the LBRY protocol
https://lbry.com
MIT License
2.57k stars 178 forks source link

lbrycrdd allows broadcasting transactions containing invalid updates #116

Closed jackrobison closed 5 years ago

jackrobison commented 6 years ago

broadcast will send transactions containing claim update outputs where the claim id is invalid. Invalid claim id sizes as well as incorrect or non-existent claim ids all get through. Attempting to broadcast such transactions should return a transaction rejected error.

bvbfan commented 6 years ago

Someone to work on?

bvbfan commented 6 years ago

@BrannonKing, @lbrynaut i don't think returning transaction rejected will be suitable for this error, what you think? I'm to find out incorrect statements compile time, rather than returning error condition in runtime. Updating claimId should not make it with incorrect size (i investigate and still not see how the size can be incorrect) or non-existing claimIds.

kauffj commented 6 years ago

@jackrobison if you're able include the specific examples of transactions you're able to broadcast but should not be able to, it'd make this ticket stronger.

bvbfan commented 6 years ago

https://github.com/lbryio/lbrycrd/pull/192

lbrynaut commented 6 years ago

Just noting that #192 does not solve the issue posted.

Edit: While it might help a user not create an invalid transaction, a tx created out of band would still be relayed/broadcasted, so it only addresses a small part of the problem.

Edit 2: On second thought, that shouldn't happen. @jackrobison Can you confirm if this issue is requesting not being able to create txs with invalid claims, or if in fact you saw txs with invalid claims being relayed?

jackrobison commented 6 years ago

@lbrynaut I had encountered this on lbryum-server, where it was running a lbrycrdd node and had made the broadcast rpc call (passing through a transaction sent to it from a lbryum client). The rpc went through without returning an error (I believe it returned a txid).

I'll try this again later today and post the results.

BrannonKing commented 5 years ago

The sendrawtransaction method relays messages even if they aren't accepted into the local memory pool. This has been repaired in the upstream bitcoin, in mid 2016.

BrannonKing commented 5 years ago

This is fixed now that we have the upstream master merged.