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

Add brotli compression for text-based inscriptions #1713

Merged
merged 60 commits into from
Nov 16, 2023
Merged

Conversation

terror
Copy link
Contributor

@terror terror commented Feb 13, 2023

Tests:

  • content encoding tag is serialized corretly. (just create inscription with content encoding field and turn it into an envelope)
  • test that content encoding appears on /inscription page
  • add unit tests for acceptencodingbrotli (test that it's correct with multiple values, spaces, qvalues, etc)
  • allow content encodings other than brotli (may need to get header value, and check if inscription.content_encoding() is present, instead of using AcceptEncodingBrotli)
  • inscriptions are compressed when passing the --compress flag
  • inscriptions are not compressed if compression doesn't make content smaller
  • test that when getting /content, server returns 406 if no content encoding header is passed when inscription is compressed
  • test that when getting /preview, server returns 406 if no content encoding header is passed when inscription is compressed

@casey
Copy link
Collaborator

casey commented Feb 13, 2023

You can add a bool to Media::TABLE, true for text formats and false for non text formats, which determines what should be compressed. Only text should be compressed for now.

@Psifour
Copy link
Contributor

Psifour commented Feb 13, 2023

Only text should be compressed for now.

I had tested the majority of our existing MIME types to verify which would benefit from brotli. The majority show gains with the 3d types being exceptionally well suited for compression.
image

src/inscription.rs Outdated Show resolved Hide resolved
src/inscription.rs Outdated Show resolved Hide resolved
@nammaki
Copy link

nammaki commented Mar 23, 2023

I hope this will be merged someday 🙏

@terror terror marked this pull request as ready for review November 14, 2023 22:45
src/envelope.rs Show resolved Hide resolved
src/inscription.rs Outdated Show resolved Hide resolved
src/media.rs Outdated Show resolved Hide resolved
src/inscription.rs Outdated Show resolved Hide resolved
src/subcommand/server/accept_encoding.rs Show resolved Hide resolved
src/subcommand/server/accept_encoding.rs Show resolved Hide resolved
src/templates/inscription.rs Outdated Show resolved Hide resolved
src/test.rs Outdated Show resolved Hide resolved
static/preview-text.js Outdated Show resolved Hide resolved
tests/wallet/inscribe.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@casey casey left a comment

Choose a reason for hiding this comment

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

Very minor nits, plus looks like tests and lint are failing.

src/subcommand/server.rs Outdated Show resolved Hide resolved
src/subcommand/server/accept_encoding.rs Outdated Show resolved Hide resolved
src/subcommand/server/accept_encoding.rs Outdated Show resolved Hide resolved
@terror
Copy link
Contributor Author

terror commented Nov 15, 2023

TODO (to get this shipped):

  • Switch to dropbox rust brotli implementation (https://docs.rs/brotli/latest/brotli/)
  • Handle these tests:
    • subcommand::server::tests::responses_are_brotlied
    • subcommand::server::tests::text_preview
    • subcommand::server::tests::text_preview_returns_error_when_content_is_not_utf8
    • subcommand::server::tests::text_preview_text_is_escaped

Copy link
Collaborator

@casey casey left a comment

Choose a reason for hiding this comment

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

🗜️

@casey casey enabled auto-merge (squash) November 16, 2023 20:22
@casey casey merged commit ab047f7 into ordinals:master Nov 16, 2023
6 checks passed
@terror terror deleted the brotli branch November 16, 2023 20:27
thedoublejay pushed a commit to sadoprotocol/ord that referenced this pull request Nov 27, 2023
* Fix lost sats bug (ordinals#2666)

* Add Hindi version of handbook (ordinals#2648)

* Remove Index::index_block_inscription_numbers (ordinals#2667)

* Hide protocol inscriptions (ordinals#2674)

* Don't color links in headers (ordinals#2678)

* Add inscription charms (ordinals#2681)

* Group rune server tests (ordinals#2685)

* Add inscription compression (ordinals#1713)

* Fix media table formatting (ordinals#2686)

* Update schema version for charms (ordinals#2687)

* Fix unbound outpoint server error (ordinals#2479)

* Add binary media type (ordinals#2671)

* Clean up install.sh (ordinals#2669)

* Add /collections Page (ordinals#2561)

* Preview font inscriptions (ordinals#2692)

* Only load used language highlight module in code preview (ordinals#2696)

* Only try to create the database if it wasn't found (ordinals#2703)

* Move postage into batch file (ordinals#2705)

* Add destination field to batch (ordinals#2701)

* Use sequence numbers database keys (ordinals#2664)

* Update redb to 1.4.0 (ordinals#2714)

* Refactor inscriptions paginations (ordinals#2715)

* Display table stats in `ord index info` (ordinals#2711)

* Use redb's recovery callback API (ordinals#2584)

* Allow setting CSP origin (ordinals#2708)

* Remove default file path from `ord index export --tsv` (ordinals#2717)

* Use icons in nav bar (ordinals#2722)

* Add Debian packaging instructions (ordinals#2725)

* Add Homebrew install instructions to readme (ordinals#2726)

* Add sat recursive endpoints with index and pagination (ordinals#2680)

* Only accept sat number in recursive endpoint (ordinals#2732)

* Fix typo in docs/src/inscriptions/metadata.md (ordinals#2731)

* Add docs for metadata recursive endpoint (ordinals#2734)

* Remove `RUNE` from <h1> on /rune (ordinals#2728)

* Add /r/children recursive endpoint (ordinals#2431)

* Add docs and examples for sat recursive endpoint (ordinals#2735)

* Ignore flaky test (ordinals#2742)

* Update docs to include all fields, including content-encoding (ordinals#2740)

* Add docs for child recursive endpoint (ordinals#2743)

* Hide JSON and .btc (ordinals#2744)

* Release 0.12.0 (ordinals#2746)

---------

Co-authored-by: raph <raphjaph@protonmail.com>
Co-authored-by: duttydeedz <142775511+duttydeedz@users.noreply.github.com>
Co-authored-by: Casey Rodarmor <casey@rodarmor.com>
Co-authored-by: liam <31192478+terror@users.noreply.github.com>
Co-authored-by: Eloc <42568538+elocremarc@users.noreply.github.com>
Co-authored-by: Julian Eager <eagr@tutanota.com>
Co-authored-by: ordinally <11798624+veryordinally@users.noreply.github.com>
Co-authored-by: Christopher Berner <me@cberner.com>
Co-authored-by: Rijndael <115941166+rot13maxi@users.noreply.github.com>
Co-authored-by: vuittont60 <81072379+vuittont60@users.noreply.github.com>
thedoublejay pushed a commit to sadoprotocol/ord that referenced this pull request Dec 1, 2023
* Fix lost sats bug (ordinals#2666)

* Add Hindi version of handbook (ordinals#2648)

* Remove Index::index_block_inscription_numbers (ordinals#2667)

* Hide protocol inscriptions (ordinals#2674)

* Don't color links in headers (ordinals#2678)

* Add inscription charms (ordinals#2681)

* Group rune server tests (ordinals#2685)

* Add inscription compression (ordinals#1713)

* Fix media table formatting (ordinals#2686)

* Update schema version for charms (ordinals#2687)

* Fix unbound outpoint server error (ordinals#2479)

* Add binary media type (ordinals#2671)

* Clean up install.sh (ordinals#2669)

* Add /collections Page (ordinals#2561)

* Preview font inscriptions (ordinals#2692)

* Only load used language highlight module in code preview (ordinals#2696)

* Only try to create the database if it wasn't found (ordinals#2703)

* Move postage into batch file (ordinals#2705)

* Add destination field to batch (ordinals#2701)

* Use sequence numbers database keys (ordinals#2664)

* Update redb to 1.4.0 (ordinals#2714)

* Refactor inscriptions paginations (ordinals#2715)

* Display table stats in `ord index info` (ordinals#2711)

* Use redb's recovery callback API (ordinals#2584)

* Allow setting CSP origin (ordinals#2708)

* Remove default file path from `ord index export --tsv` (ordinals#2717)

* Use icons in nav bar (ordinals#2722)

* Add Debian packaging instructions (ordinals#2725)

* Add Homebrew install instructions to readme (ordinals#2726)

* Add sat recursive endpoints with index and pagination (ordinals#2680)

* Only accept sat number in recursive endpoint (ordinals#2732)

* Fix typo in docs/src/inscriptions/metadata.md (ordinals#2731)

* Add docs for metadata recursive endpoint (ordinals#2734)

* Remove `RUNE` from <h1> on /rune (ordinals#2728)

* Add /r/children recursive endpoint (ordinals#2431)

* Add docs and examples for sat recursive endpoint (ordinals#2735)

* Ignore flaky test (ordinals#2742)

* Update docs to include all fields, including content-encoding (ordinals#2740)

* Add docs for child recursive endpoint (ordinals#2743)

* Hide JSON and .btc (ordinals#2744)

* Release 0.12.0 (ordinals#2746)

* Hide all text (ordinals#2753)

* Add batch to preview command (ordinals#2752)

* Add stuttering curse (ordinals#2745)

* Batch inscribe on same sat (ordinals#2749)

* Allow setting the sat to inscribe (ordinals#2765)

* Select further away coins which meet target (ordinals#2724)

* Fix typos (ordinals#2768)

* Add ability to specify sat to batch inscribe (ordinals#2770)

* Add commands to etch and list runes (ordinals#2544)

* Set CSP origin in deploy script (ordinals#2764)

Co-authored-by: raph <raphjaph@protonmail.com>

* Add `public` to /content Cache-Control headers (ordinals#2773)

* Release 0.12.1 (ordinals#2776)

* Bless cursed inscriptions after Jubilee height (ordinals#2656)

* Hide /content/<INSCRIPTION_ID> HTML inscriptions (ordinals#2778)

* Release 0.12.2 (ordinals#2780)

* fix(test): error test from version 0.12.2

---------

Co-authored-by: raph <raphjaph@protonmail.com>
Co-authored-by: duttydeedz <142775511+duttydeedz@users.noreply.github.com>
Co-authored-by: Casey Rodarmor <casey@rodarmor.com>
Co-authored-by: liam <31192478+terror@users.noreply.github.com>
Co-authored-by: Eloc <42568538+elocremarc@users.noreply.github.com>
Co-authored-by: Julian Eager <eagr@tutanota.com>
Co-authored-by: ordinally <11798624+veryordinally@users.noreply.github.com>
Co-authored-by: Christopher Berner <me@cberner.com>
Co-authored-by: Rijndael <115941166+rot13maxi@users.noreply.github.com>
Co-authored-by: vuittont60 <81072379+vuittont60@users.noreply.github.com>
Co-authored-by: gmart7t2 <49558347+gmart7t2@users.noreply.github.com>
Co-authored-by: xiaolou86 <20718693+xiaolou86@users.noreply.github.com>
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