fix(engine)!: hard limit on template binary size - #1640
Merged
sdbondi merged 1 commit intoNov 11, 2025
Conversation
Contributor
WalkthroughThis pull request refactors the transaction template validation architecture by removing Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Transaction Handler
participant TxBuilder as TransactionBuilder
participant Instruction as PublishTemplate<br/>(Instruction)
participant TemplateBlob as TemplateBlob<br/>(MaxBytes)
participant Processor as TransactionProcessor
participant Runtime as Runtime
Note over Caller,Runtime: New flow: Earlier validation via try_into()
Caller->>TxBuilder: publish_template(binary: Vec<u8>)
TxBuilder->>TemplateBlob: binary.try_into()
alt Size within limit
TemplateBlob-->>TxBuilder: Ok(TemplateBlob)
TxBuilder->>Instruction: PublishTemplate { binary: TemplateBlob }
Instruction-->>Caller: Transaction with validated blob
Caller->>Processor: execute(transaction)
Processor->>Runtime: publish_template(binary: TemplateBlob)
Runtime-->>Processor: InstructionResult
else Size exceeds limit
TemplateBlob-->>TxBuilder: Err (size validation failed)
TxBuilder-->>Caller: Error: invalid_params
end
Note over Caller,Runtime: Old flow (removed): Config-based validation
Note over Caller,Runtime: TransactionProcessorConfig and size field removed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Areas requiring extra attention:
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
applications/tari_app_utilities/src/transaction_executor.rs(1 hunks)applications/tari_indexer/src/bootstrap.rs(2 hunks)applications/tari_indexer/src/dry_run/processor.rs(3 hunks)applications/tari_validator_node/src/bootstrap.rs(0 hunks)applications/tari_walletd/src/handlers/transaction.rs(1 hunks)crates/consensus/src/consensus_constants.rs(0 hunks)crates/consensus_tests/src/consensus.rs(2 hunks)crates/consensus_tests/src/support/harness.rs(0 hunks)crates/engine/src/transaction/config.rs(0 hunks)crates/engine/src/transaction/error.rs(0 hunks)crates/engine/src/transaction/mod.rs(0 hunks)crates/engine/src/transaction/processor.rs(7 hunks)crates/engine/tests/publish_template.rs(3 hunks)crates/engine_types/src/limits.rs(2 hunks)crates/p2p/src/conversions/transaction.rs(2 hunks)crates/template_lib_types/src/max_bytes.rs(2 hunks)crates/template_manager/src/implementation/service.rs(1 hunks)crates/template_test_tooling/src/template_test.rs(2 hunks)crates/transaction/src/builder/mod.rs(2 hunks)crates/transaction/src/transaction.rs(1 hunks)crates/transaction/src/v1/instruction.rs(4 hunks)
💤 Files with no reviewable changes (6)
- crates/engine/src/transaction/error.rs
- crates/engine/src/transaction/mod.rs
- crates/consensus_tests/src/support/harness.rs
- crates/consensus/src/consensus_constants.rs
- crates/engine/src/transaction/config.rs
- applications/tari_validator_node/src/bootstrap.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-04T10:10:24.258Z
Learnt from: sdbondi
Repo: tari-project/tari-ootle PR: 1629
File: applications/tari_walletd/src/handlers/accounts.rs:1001-1002
Timestamp: 2025-11-04T10:10:24.258Z
Learning: In applications/tari_walletd/src/handlers/accounts.rs, the expect() on Memo::new_pay_ref_and_bytes_truncate at line 1002 is safe and intentional. PayRef is validated to be at most 64 bytes during address decoding (PayRef::MAX_LEN = 64), and the function only returns None if payref exceeds 252 bytes (Memo::MAX_BYTES_LENGTH - 1). Since 64 < 252, None is impossible with a valid PayRef.
Applied to files:
crates/engine/tests/publish_template.rsapplications/tari_walletd/src/handlers/transaction.rs
🧬 Code graph analysis (9)
crates/transaction/src/builder/mod.rs (3)
crates/engine/src/transaction/processor.rs (1)
publish_template(451-458)crates/engine/src/runtime/impl.rs (1)
publish_template(2713-2744)crates/engine/src/runtime/mod.rs (1)
publish_template(191-191)
crates/template_lib_types/src/max_bytes.rs (3)
crates/template_lib_types/src/hash.rs (1)
as_slice(58-60)crates/state_store_rocksdb/src/codecs/small_bytes.rs (1)
as_slice(59-61)crates/template_lib_types/src/max_string.rs (1)
new_checked(24-31)
crates/p2p/src/conversions/transaction.rs (5)
crates/engine/src/transaction/processor.rs (1)
publish_template(451-458)crates/transaction/src/builder/mod.rs (1)
publish_template(290-292)crates/engine/src/runtime/impl.rs (1)
publish_template(2713-2744)crates/engine/src/runtime/mod.rs (1)
publish_template(191-191)integration_tests/src/template.rs (1)
publish_template(30-85)
crates/transaction/src/v1/instruction.rs (1)
bindings/src/types/Instruction.ts (1)
Instruction(18-45)
applications/tari_walletd/src/handlers/transaction.rs (1)
applications/tari_walletd/src/handlers/helpers.rs (1)
invalid_params(161-172)
crates/consensus_tests/src/consensus.rs (3)
crates/engine_types/src/hashing.rs (1)
hash_template_code(55-57)crates/transaction/src/builder/mod.rs (1)
new(54-60)crates/engine_types/src/published_template.rs (1)
from_author_and_binary_hash(43-49)
applications/tari_indexer/src/dry_run/processor.rs (2)
crates/engine/src/state_store/bootstrap.rs (1)
new_memory_store(6-8)crates/engine/src/transaction/processor.rs (1)
new(91-107)
applications/tari_indexer/src/bootstrap.rs (2)
applications/tari_app_utilities/src/fee_tables.rs (1)
get_fee_table_by_network(26-35)applications/tari_indexer/src/dry_run/processor.rs (1)
new(57-70)
crates/engine/src/transaction/processor.rs (4)
crates/transaction/src/builder/mod.rs (1)
publish_template(290-292)crates/engine/src/runtime/impl.rs (1)
publish_template(2713-2744)crates/engine/src/runtime/mod.rs (1)
publish_template(191-191)crates/engine/src/wasm/module.rs (1)
load_template_from_code(65-100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test
- GitHub Check: check stable
- GitHub Check: check nightly
- GitHub Check: machete
- GitHub Check: clippy
🔇 Additional comments (3)
crates/template_manager/src/implementation/service.rs (1)
401-409: Documentation-only change—comment appropriately describes placeholder behavior.The comment at line 402 is concise and clearly explains why
"<unknown>"is used as a placeholder template name before download completion. This aligns with the actual code behavior.crates/consensus_tests/src/consensus.rs (2)
1362-1372: LGTM! Clean refactor to precompute the hash.The changes improve code clarity by precomputing the template binary hash once and reusing it throughout the test. The
try_into().unwrap()on line 1364 correctly converts the raw WASM bytes toTemplateBlob, enforcing the new size limit. Since this test uses a known fixture (state.wasm), the unwrap is appropriate—if the fixture ever exceeds the limit, the test will fail fast, which is the desired behavior.
1415-1415: LGTM! Assertion correctly uses the precomputed hash.Consistent with the earlier refactor, the assertion now compares against the
expected_binary_hashdefined at line 1362, improving test clarity and avoiding unnecessary recomputation.
Comment on lines
85
to
87
| .fee_transaction_pay_from_component(account_address, 200_000) | ||
| .publish_template(random_wasm_binary) | ||
| .publish_template(random_wasm_binary.try_into().unwrap()) | ||
| .build_and_seal(&account_key), |
Contributor
There was a problem hiding this comment.
Don’t unwrap the oversize template blob
TemplateBlob now enforces the 2 MiB engine limit, so random_wasm_binary.try_into() returns Err for the 6 MiB sample. The unwrap() added here will panic and the test never reaches the engine rejection assertions. Please update this test to handle the Err (e.g. assert that the conversion fails before building the transaction, or restructure the scenario to exercise the serialized-transaction path) so the suite keeps running.
🤖 Prompt for AI Agents
In crates/engine/tests/publish_template.rs around lines 85 to 87, the test
currently calls random_wasm_binary.try_into().unwrap() which will panic because
TemplateBlob enforces a 2 MiB limit and the sample is 6 MiB; replace the unwrap
with proper handling: call random_wasm_binary.try_into(), match the Result, and
if it is Err assert that the conversion fails with the expected oversize/limit
error (so the test records the failure before building the transaction),
otherwise (if you want to test the engine rejection path) use a smaller wasm
sample or restructure the test to serialize and submit a transaction that
reaches the engine; ensure no unwrap is left so the test never panics.
Merged
sdbondi
added a commit
that referenced
this pull request
Nov 11, 2025
Description --- Forgot to push fixes in #1640
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
fix(engine)!: hard limit on template binary size
Motivation and Context
Transaction cannot be constructed (in Rust code) with a binary that exceeds the pre-defined engine limit (currently 2MiB)
Serialized transactions that exceed this limit will be rejected at the decoding level.
How Has This Been Tested?
Existing tests
What process can a PR reviewer use to test or verify this change?
Breaking Changes
BREAKING CHANGE: JSON encoding for published binary data uses base64 encoding for compactness. This is a breaking change for any JSON-api that accepts transactions
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor