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

Do not split runestone payload into MAX_SCRIPT_ELEMENT_SIZE pushes #4036

Merged
merged 3 commits into from
Nov 3, 2024

Conversation

casey
Copy link
Collaborator

@casey casey commented Oct 29, 2024

I believe the MAX_SCRIPT_ELEMENT_SIZE limit is only checked when outputs are spent. Since runestones begin with an OP_RETURN, they can never be spent, so we do not need to chunk the payload.

It's unfortunate that I'm only realizing this now, since if I had known it earlier, we could have required the payload to be a single data push, since without the MAX_SCRIPT_ELEMENT_SIZE limit, there's no reason to accept multiple data pushes in the payload.

This is marked as a draft PR, because although I think it's correct, I want to actually test it against Bitcoin Core to confirm before merging.

@casey casey force-pushed the dont-chunk-runestones branch from a07bd67 to 8b7367b Compare November 3, 2024 19:37
@casey
Copy link
Collaborator Author

casey commented Nov 3, 2024

Confirmed this locally with a split which generated a 4550 byte OP_RETURN. OP_RETURNS are totally uninterpreted and unexecuted. The script does not even need to be valid, and the script element size of 520 byte is unchecked.

@casey casey marked this pull request as ready for review November 3, 2024 19:39
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

LGTM

@casey casey merged commit 12fdfdf into ordinals:master Nov 3, 2024
5 checks passed
@casey casey deleted the dont-chunk-runestones branch November 3, 2024 23:05
@gmart7t2
Copy link
Contributor

gmart7t2 commented Nov 4, 2024

I believe the MAX_SCRIPT_ELEMENT_SIZE limit is only checked when outputs are spent

I can confirm that.

I had to change MAX_OP_RETURN_RELAY to get sendrawtransaction to accept an OP_RETURN longer than 83 bytes but that's the only change I had to make. Then I was able to create a 200 edict runestone with a 874 byte OP_PUSHDATA2:

$ bitcoin-cli -regtest getrawtransaction a6c3de3d9b9fb0f849816e7127987aef4e7245af0a041f1feebde089ea5481dc 1 | jq -r .vout[0].scriptPubKey.hex
6a5d4d6a0300c5090164000000640100006402000064030000640400006405000064060000640700006408000064090000640a0000640b0000640c0000640d0000640e0000640f000064100000641100006412000064130000641400006415000064160000641700006418000064190000641a0000641b0000641c0000641d0000641e0000641f000064200000642100006422000064230000642400006425000064260000642700006428000064290000642a0000642b0000642c0000642d0000642e0000642f000064300000643100006432000064330000643400006435000064360000643700006438000064390000643a0000643b0000643c0000643d0000643e0000643f000064400000644100006442000064430000644400006445000064460000644700006448000064490000644a0000644b0000644c0000644d0000644e0000644f000064500000645100006452000064530000645400006455000064560000645700006458000064590000645a0000645b0000645c0000645d0000645e0000645f000064600000646100006462000064630000646400006465000064660000646700006468000064690000646a0000646b0000646c0000646d0000646e0000646f000064700000647100006472000064730000647400006475000064760000647700006478000064790000647a0000647b0000647c0000647d0000647e0000647f00006480010000648101000064820100006483010000648401000064850100006486010000648701000064880100006489010000648a010000648b010000648c010000648d010000648e010000648f0100006490010000649101000064920100006493010000649401000064950100006496010000649701000064980100006499010000649a010000649b010000649c010000649d010000649e010000649f01000064a001000064a101000064a201000064a301000064a401000064a501000064a601000064a701000064a801000064a901000064aa01000064ab01000064ac01000064ad01000064ae01000064af01000064b001000064b101000064b201000064b301000064b401000064b501000064b601000064b701000064b801000064b901000064ba01000064bb01000064bc01000064bd01000064be01000064bf01000064c001000064c101000064c201000064c301000064c401000064c501000064c601000064c701
$ python
>>> 1748/2
874.0
>>> 0x036a
874

That's 6a (op_return) 5d (op_13) 4d (op_pushdata2) 6a03 (874 bytes) ...

@casey
Copy link
Collaborator Author

casey commented Nov 4, 2024

Nice, thanks for confirming! I did too, but always nice to double check, since it would be catastrophic if I fucked this one up.

danadi7 pushed a commit to sadoprotocol/ord that referenced this pull request Nov 20, 2024
* Change test-bitcoincore-rpc to mockcore in README.md (ordinals#3842)

* Commit twice to work around redb off-by-one bug (ordinals#3856)

* Release 0.19.1 (ordinals#3864)

- Bump version: 0.19.0 → 0.19.1
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Update Portuguese Translation pt.po (ordinals#3837)

* Updated Chinese translation  (ordinals#3881)

* Fix rune links for runes with no symbol (ordinals#3849)

* Suppress printing sat_ranges by default (ordinals#3867)

* Re-enter beta (ordinals#3884)

* Update pointer specification (ordinals#3861)

* Clarify that unused runes tags should not be used (ordinals#3885)

* Migrate object.rs to snafu error handling (ordinals#3858)

* Make index settings harder to misuse (ordinals#3893)

* Don't unnecessarily insert into utxo cache when indexing addresses (ordinals#3894)

* Remove trailing space from runes specification (ordinals#3896)

* Serve responses with cross origin isolation headers (ordinals#3898)

* List all Bitcoin Core wallets (ordinals#3902)

* Make first first and last sat in range clickable (ordinals#3903)

* Migrate Outgoing to SnafuError (ordinals#3854)

* Update Bitcoin Core deploy to 27.1 (ordinals#3912)

* Add sat_balance to address API (ordinals#3905)

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

* Add Dutch translation to Ordinals Handbook (ordinals#3907)

* Migrate chain.rs to snafu error (ordinals#3904)

* Bump version to 0.20.0-dev (ordinals#3916)

* Revert "Serve responses with cross origin isolation headers" (ordinals#3920)

* Remove inscription content type counts from /status page (ordinals#3922)

* Unified OUTPOINT_TO_UTXO_ENTRY table (ordinals#3915)

- Upgrade `redb` to 2.1.1
- Remove `--index-spent-sats`
- Remove redundant pointer handling in `index_inscriptions()`
- Fix incorrect `is_output_spent()` results when not using `--index-sats`
- Unify UTXO index data in `OUTPOINT_TO_UTXO_ENTRY` table
- Read addresses from index when exporting

* Add address field to `/r/inscription/:id` (ordinals#3891)

* Add inscriptions and runes details to address API endpoint (ordinals#3924)

* Release 0.20.0 (ordinals#3928)

- Bump ord version: 0.19.1 → 0.20.0
- Bump ordinals version: 0.0.9 → 0.0.10
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Bump version to 0.20.0-dev (ordinals#3929)

* Add test to remind us to fix the UtxoEntry redb type name (ordinals#3934)

* Put AddressInfo into api module (ordinals#3933)

* Fix clippy lint (ordinals#3937)

* Add inscription index to /status (ordinals#3938)

* Remove unnecessary symbols in docs/src/guides/testing.md (ordinals#3945)

* Add inscription examples to handbook (ordinals#3769)

* Allow scrolling in iframe (ordinals#3947)

* Skip serializing None in batch::File (ordinals#3943)

* Fix /output page (ordinals#3948)

* Add `/satpoint/<SATPOINT>` endpoint (ordinals#3949)

* Don't log RPC connections to bitcoind (ordinals#3952)

* Start indexing at correct block height (ordinals#3956)

* Fix output API struct (ordinals#3957)

* Remove dependency on `ord-bitcoincore-rpc` crate (ordinals#3959)

* Keep sat ranges in low-level format (ordinals#3963)

* Implement burn for wallet command (ordinals#3437)

* Add multi parent support to wallet (ordinals#3228)

* Get parents using `as_slice` instead of converting to `Vec` (ordinals#3972)

* Rename parents_values -> parent_values (ordinals#3973)

* Fix non-existant output lookup (ordinals#3968)

* Release 0.20.1 (ordinals#3975)

* Refactor burn command (ordinals#3976)

* Remove regtest.ordinals.net just recipes (ordinals#3978)

* Add `ord verify` (ordinals#3906)

* Release 0.21.0 (ordinals#3997)

- Bump version: 0.20.1 → 0.21.0
- Update changelog
- Update changelog contributor credits
- Update dependencies

* Remove /runes/balances API endpoint (ordinals#3980)

* Update rust-bitcoin in ord (ordinals#3962)

* Revert redb to 2.1.3 (ordinals#4003)

* Release 0.21.1 (ordinals#4006)

* Update Bitcoin Core install script (ordinals#4007)

* Remove pre-alpha warning from ord help (ordinals#4011)

* Change mint progress to `mints / terms.cap` (ordinals#4012)

* Only show rune mint progress during mint (ordinals#4013)

* Show if JSON API is enabled on /status (ordinals#4014)

* Update JSON-API & Recursive documentation (ordinals#3984)

* Document POST method for /inscriptions (ordinals#4017)

* Add authors to Handbook (ordinals#4018)

* Add more info to `wallet outputs` (ordinals#4019)

* Add `wallet addresses` (ordinals#4005)

* Add BIP322 `wallet sign` (ordinals#3988)

* Add `/r/undelegated-content/<INSCRIPTION_ID>` (ordinals#3932)

* Show total child count (ordinals#4009)

* Create change output when inputs containing non-outgoing runes are selected (ordinals#4028)

* Release 0.21.2 (ordinals#4029)

- Bump version: 0.21.1 → 0.21.2
- Update changelog
- Update changelog contributor credits

* Allow fallback for satpoints and addresses (ordinals#4033)

* Un-pin redb dependency and update to 2.2.0 (ordinals#4032)

* Add `ord wallet split` command for splitting utxos (ordinals#4030)

* Hide image preview and thumbnail scrollbars (ordinals#4042)

* Rescan wallet on restore (ordinals#4041)

* Do not chunk runestone data pushes (ordinals#4036)

* Add simple taproot HD wallet to mockcore (ordinals#4038)

* Collapse long strings in HTML (ordinals#4053)

* BIP322 sign file (ordinals#4026)

* Identify collapsible nodes with class=collapse (ordinals#4055)

* Add assert_html function (ordinals#4058)

* Allow including metadata when burning inscriptions (ordinals#4045)

* Get output information by address (ordinals#4056)

* Document split command (ordinals#4062)

* Add palindrome charm (ordinals#4064)

* Allow restoring wallet with custom timestamp (ordinals#4065)

* Release 0.21.3 (ordinals#4059)

- Bump version: 0.21.2 → 0.21.3
- Update changelog
- Update changelog contributor credits
- Update dependencies
- Pin `bitcoin` to 0.32.3
- Downgrade `bip322` to 0.0.8

* Add Cargo.lock

---------

Co-authored-by: Anchor <49644098+TheHeBoy@users.noreply.github.com>
Co-authored-by: Casey Rodarmor <casey@rodarmor.com>
Co-authored-by: 0xArtur <metaverseartur@gmail.com>
Co-authored-by: Dr.JingLee <95461562+DrJingLee@users.noreply.github.com>
Co-authored-by: nine <118634361+cryptoni9n@users.noreply.github.com>
Co-authored-by: Bohdan Cryptolions <37701692+ansigroup@users.noreply.github.com>
Co-authored-by: raph <raphjaph@protonmail.com>
Co-authored-by: Patrick Collins <patrick@collinatorstudios.com>
Co-authored-by: Tibebtc <tibedekock@live.nl>
Co-authored-by: partialord <178683722+partialord@users.noreply.github.com>
Co-authored-by: Eloc <42568538+elocremarc@users.noreply.github.com>
Co-authored-by: twosatsmaxi <vashalpesh@gmail.com>
Co-authored-by: tiaoxizhan <tiaoxizhan@outlook.com>
Co-authored-by: onchainguy <1436535+onchainguy-btc@users.noreply.github.com>
Co-authored-by: lifofifo <134870335+lifofifoX@users.noreply.github.com>
Co-authored-by: Arik <arik-so@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.

3 participants