Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dex/encode: allow data longer than 65535 bytes #1620

Merged
merged 2 commits into from
May 25, 2022

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented May 17, 2022

Relating to the issue hit in #1105 (comment), where the counter tx data was too large on account of the tx having many funding coins.

Retrofit the blob encoding and decoding to allow byte slices up to
0x00fe_ffff (16711679) long.  The decoding scheme detects a uint32
length when the top two bytes decoded as big endian uint16 indicate a
value less than 255.

This is a bit of a cludge, but it seems to be working, so I figure we could discuss it.

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

dex/encode/encode.go Show resolved Hide resolved
@chappjc chappjc force-pushed the larger-blobs branch 2 times, most recently from 3b53dc5 to 43d9aef Compare May 20, 2022 21:39
Copy link
Contributor

@martonp martonp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

dex/encode/encode.go Show resolved Hide resolved
Comment on lines +194 to +197
// cannot switch to uint32 at 65535 itself since it is possible
// there is data of exactly that length already stored using just
// two bytes to encode the length. Thus, the decoder should inspect
Copy link
Member

@buck54321 buck54321 May 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah crap. It it wasn't for this, we could just us the varint stuff from wire.

Retrofit the blob encoding and decoding to allow byte slices up to
0x00fe_ffff (16711679) long.  The decoding scheme detects a uint32
length when the top two bytes decoded as big endian uint16 indicate a
value less than 255.
@chappjc chappjc added this to the 0.5 milestone May 25, 2022
@chappjc
Copy link
Member Author

chappjc commented May 25, 2022

Well, good to go then, although despite my best efforts on this one, I still feel nervous that there's a gotcha somewhere despite it testing well. We have a bit until 0.5 so I guess we'll just see how this does with more use.

@chappjc chappjc merged commit b41da10 into decred:master May 25, 2022
@chappjc chappjc deleted the larger-blobs branch May 25, 2022 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants