simpleledger / slp-specifications

Simple Ledger Protocol (SLP) Specifications
MIT License
57 stars 40 forks source link

Add procedures, change the order of pushes #37

Closed DesWurstes closed 3 years ago

DesWurstes commented 3 years ago

These changes simplify the code

@dagurval Could you too please review the wording? Is there an ambiguity I can clear up in this?

dagurval commented 3 years ago

The changes describing the scripts and push payloads look good to me. I'm not familiar enough with the protocol to comment on the procedure updates.

DesWurstes commented 3 years ago

"Padding shall be added to satisfy the conditions that the redeem script above is valid and that every P2SH push is at least 220 bytes. While reading, the bytes after file_byte_count_int in the metadata will be discarded."

Won't adding padding possibly cause the uploaded file to be corrupted?

It's always the last bytes to be padded which will be discarded. Padding is a must because:

The reading code can be generalized to reading every byte array that is longer than a signature. So if every push is larger than it, if people use alternative pushing scripts the downloader would still work.

However if there are smaller pushes, other than the downloader problem above, then the standard redeem will be invalid because of empty pushes. There might even be standardness problems due to possible single-byte pushes if we try to distribute the remaining data over remaining pushes.

then OP_RETURNs must be used?

Because of the problem above. If a P2SH push is being filled that means the transaction already has an OP_RETURN, while the next transaction would have a metadata output. If we try to use OP_RETURN we would need extra code to check 1) Fits in OP_RETURN 2) Fits in the space left in metadata push while the probability below is not significant (for the 0x01 protocol it is 99% that there would be a partial OP_RETURN).

Why not just say if there is < X bytes to be pushed then OP_RETURNs must be used?

If we need to push 1000 bytes more that doesn't fit into OP_RETURN; instead of specially handling the case padding has worked well so far over Blockupload.io.

Is there a reason why the final transaction's metadata shouldn't be allowed to contain a data chunk?

There is a probability of 14% that the remaining data would fit in OP_RETURN (while the rest requiring a P2SH output) assuming the metadata is empty, even less otherwise.

Do you find it worth handling this case? This currently minimizes the number of edge cases.

jcramer commented 3 years ago

How does a downloader know what is padding and what is bytes associated with the end of the original file. What if the end of the file is 00..00? How will the downloader verify the hash?

Also, its probably obvious, but just reiterating that the hash256 in the metadata is for the file, not the file with padding added.

DesWurstes commented 3 years ago

How does a downloader know what is padding and what is bytes associated with the end of the original file. What if the end of the file is 00..00? How will the downloader verify the hash?

A buffer is created with length. The order of reading chunks is well-defined. Chunks are copied there. Bytes that don't fit (padding bytes) are dropped. The hash is calculated from this buffer. So the padding bytes don't affect the hash or the final buffer.

Also, its probably obvious, but just reiterating that the hash256 in the metadata is for the file, not the file with padding added.

Yes!

EDIT: Also, I have decided to cancel "Is there a reason why the final transaction's metadata shouldn't be allowed to contain a data chunk?" because the download code allows that already without any changes and but it should be optional to the uploader. Mine now does that, looking forward to opening a Pull Request once https://github.com/simpleledger/BitcoinFilesJS/pull/20 is merged.