simpleledger / slp-specifications

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

No maximum length to some SLP fields can cause issues in theory #14

Closed Greg-Griffith closed 5 years ago

Greg-Griffith commented 5 years ago

As posted on reddit (modified to address some feedback already gotten):

In the current SLP spec , , and , henceforth name_fields, have fields where there is no maximum size for the names (they may be infinite bytes long).

In theory this is ok, but not having a max length on the field makes parsing this data impossible without making assumptions.

As pointed out, data in SLP needs to handle 4 forms, defined in: Bitcoin script allows a given byte array to be pushed in various ways, and we allow this in SLP as well. For example, it is valid to push a 4-byte chunk (like the Lokad ID) in four different ways: 0x04 [chunk] (size then chunk), 0x4c 0x04 [chunk] (OP_PUSHDATA1 then size then chunk), 0x4d 0x04 0x00 [chunk] (OP_PUSHDATA2 then size then chunk), or 0x4e 0x04 0x00 0x00 0x00 [chunk] (OP_PUSHDATA4 then size then chunk).

From purely a spec standpoint, this is problematic. There is no valid way to implement the full range of spec valid values. The spec is relying on the BCH implementation and OP_RETURN rules to ensure everything works out.

I think the spec should be adjusted to have 2^32-1 be the max value of the name_fields.

Afterthought: After thinking about it, i realize this isn't really a realistic issue and is essentially a nitpick. I dont think anyone would, without intent to specifically cause a fuss, actually try to make one of the name_fields that long. It mostly just felt wrong to see infinity as a valid length in a technical specification.

Edit: Most of my concern came from an incorrect calculation for OP_PUSHDATA4, i did 256 * 4 not 256 ^ 4

markblundeberg commented 5 years ago

Ah OK, I think I see where you're coming from now. :+1:

Yes, the upper limit would be better stated as 2^32-1. In other words even if the 1 MB per transaction limit were lifted and a 4 GB push were possible, we would still consider it for being valid SLP, just on principle. :-D