Fault proving: Populate ContractsRawCode when processing Create Transaction in global merkle root crate #2652

acerone85 · 2025-01-30T16:48:42Z

Linked Issues/PRs

Closes #2587

Description

Checklist

Breaking changes are clearly marked as such in the PR description and changelog
New behavior is reflected in tests
The specification matches the implemented behavior (link update PR if changes are needed)

Before requesting review

I have reviewed the code myself
I have created follow-up issues caused by this PR and linked them here

After merging, notify other teams

[Add or remove entries as needed]

Rust SDK
Sway compiler
Platform documentation (for out-of-organization contributors, the person merging the PR will do this)
Someone else?

netrome

Thanks for implementing this. Overall looks good but there are two tiny blockers I can see:

A dangling todo that should reference an issue.
An as conversion in production code.

If these are addressed I'd happily approve this PR.

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

xgreenx · 2025-01-31T18:02:00Z

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

+        let contract_id = tx
+            .metadata()
+            .as_ref()
+            .map(|metadata| metadata.body.contract_id)
+            .ok_or(anyhow::anyhow!(
+                "Create transaction does not have a contract ID in its metadata"
+            ))?;


I wouldn't rely on existence of the contract_id in metadata. I would suggest to use contract_id from ContractCreated output

Done in 0f91ceb, but I'm curious to know why the metadata is unreliable.

The base branch was changed.

netrome

Just a nit :)

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

MitchTurner · 2025-02-04T17:33:29Z

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

+    }
+
+    // TODO: https://github.com/FuelLabs/fuel-core/issues/2654
+    // This code is copied from the executor. We should refactor it to be shared.


Maybe. It's not super obvious to me who should own this function. Maybe add it to where TransactionBuilder is maintained. Or add the functionality to the TransactionBuilder :)

There is a separate PR (#2654) where I add it to fuel-core-types behind the test-helpers feature. Happy to discuss if this is the right place there.

MitchTurner · 2025-02-04T17:34:28Z

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

+
+    // TODO: https://github.com/FuelLabs/fuel-core/issues/2654
+    // This code is copied from the executor. We should refactor it to be shared.
+    fn create_contract(bytecode: &[u8], rng: &mut impl rand::RngCore) -> Create {


nit: rename this create_contract_tx

Done: e6aadd5

MitchTurner · 2025-02-04T17:35:44Z

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

+    // CreateBody is not publicly exported, so we need to use a generic type
+    // and ensure that all trait bounds are satisfied


Is this comment still relevant?

Nope, removed in be996d8

Co-authored-by: Mårten Blankfors <marten@blankfors.se>

## Version 0.41.6 ### Added - [2668](#2668): Expose gas price service test helpers - [2621](#2598): Global merkle root storage updates process upgrade transactions. - [2650](#2650): Populate `ProcessedTransactions` table in global merkle root storage. - [2667](#2667): Populate `Blobs` table in global merkle root storage. - [2652](#2652): Global Merkle Root storage crate: Add Raw contract bytecode to global merkle root storage when processing Create transactions. ### Fixed - [2673](#2673): Change read behavior on the InMemoryTransaction to use offset and allow not equal buf size (fix CCP and LDC broken from FuelLabs/fuel-vm#847)

xgreenx · 2025-02-06T18:31:18Z

crates/fraud_proofs/global_merkle_root/storage/src/update.rs

+    fn process_create_transaction(&mut self, tx: &Create) -> anyhow::Result<()> {
+        let bytecode_witness_index = tx.bytecode_witness_index();
+        let witnesses = tx.witnesses();
+        let bytecode = witnesses[usize::from(*bytecode_witness_index)].as_vec();


If bytecode_witness_index is malicious, it will panic

Interesting that clippy missed this 🤔 thanks for spotting!

Created hotfix PR #2677

acerone85 added the fault-proving label Jan 30, 2025

acerone85 self-assigned this Jan 30, 2025

acerone85 requested review from xgreenx, netrome, rymnc, Dentosal and MitchTurner as code owners January 30, 2025 16:48

netrome requested changes Jan 31, 2025

View reviewed changes

crates/fraud_proofs/global_merkle_root/storage/src/update.rs Outdated Show resolved Hide resolved

crates/fraud_proofs/global_merkle_root/storage/src/update.rs Outdated Show resolved Hide resolved

crates/fraud_proofs/global_merkle_root/storage/src/update.rs Outdated Show resolved Hide resolved

acerone85 commented Jan 31, 2025

View reviewed changes

crates/fraud_proofs/global_merkle_root/storage/src/update.rs Outdated Show resolved Hide resolved

acerone85 force-pushed the 2587-fault_provingglobal_roots-populate-contractsrawcode-table branch from 7f0ff15 to ebe1f20 Compare January 31, 2025 14:06

netrome previously approved these changes Jan 31, 2025

View reviewed changes

xgreenx reviewed Jan 31, 2025

View reviewed changes

netrome force-pushed the 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates branch from 2457f06 to bbf6361 Compare January 31, 2025 20:51

netrome requested review from rafal-ch and AurelienFT as code owners January 31, 2025 20:51

Base automatically changed from 2582-fault_provingglobal_roots-initial-test-suite-for-storage-updates to master January 31, 2025 21:11

acerone85 force-pushed the 2587-fault_provingglobal_roots-populate-contractsrawcode-table branch from ebe1f20 to 4177a75 Compare February 3, 2025 11:25

netrome previously approved these changes Feb 3, 2025

View reviewed changes

crates/fraud_proofs/global_merkle_root/storage/src/update.rs Outdated Show resolved Hide resolved

acerone85 dismissed netrome’s stale review via 14ca97b February 3, 2025 18:41

acerone85 force-pushed the 2587-fault_provingglobal_roots-populate-contractsrawcode-table branch from d103ddb to ee3edba Compare February 4, 2025 10:26

rymnc requested review from xgreenx and netrome February 4, 2025 14:48

netrome previously approved these changes Feb 4, 2025

View reviewed changes

MitchTurner reviewed Feb 4, 2025

View reviewed changes

acerone85 dismissed netrome’s stale review via e6aadd5 February 5, 2025 09:41

acerone85 force-pushed the 2587-fault_provingglobal_roots-populate-contractsrawcode-table branch from d0572ee to e6aadd5 Compare February 5, 2025 09:41

acerone85 and others added 3 commits February 5, 2025 12:44

Add Contract Bytecode when processing a Create Transaction

bf0bfd0

Changelog

a8cf21a

Update crates/fraud_proofs/global_merkle_root/storage/src/update.rs

e4a5f9c

Co-authored-by: Mårten Blankfors <marten@blankfors.se>

acerone85 and others added 10 commits February 5, 2025 12:44

Update crates/fraud_proofs/global_merkle_root/storage/src/update.rs

d554d38

Co-authored-by: Mårten Blankfors <marten@blankfors.se>

Update crates/fraud_proofs/global_merkle_root/storage/src/update.rs

dfcd873

Formatting

be67a2b

Use transaction outputs to retrieve contract id

587b137

Fix Clippy

82d3cf1

Update crates/fraud_proofs/global_merkle_root/storage/src/update.rs

a05d94c

Co-authored-by: Mårten Blankfors <marten@blankfors.se>

filter_next -> find

fef57b2

Remove stale comment

3314ab9

create_contract -> create_contract_tx

9f5674b

Fix newlines after rebase

6bc31c6

acerone85 force-pushed the 2587-fault_provingglobal_roots-populate-contractsrawcode-table branch from e6aadd5 to 6bc31c6 Compare February 5, 2025 12:47

netrome approved these changes Feb 5, 2025

View reviewed changes

rymnc approved these changes Feb 5, 2025

View reviewed changes

netrome merged commit 05538a5 into master Feb 5, 2025
33 checks passed

netrome deleted the 2587-fault_provingglobal_roots-populate-contractsrawcode-table branch February 5, 2025 13:16

AurelienFT mentioned this pull request Feb 5, 2025

Release 0.41.6 #2674

Merged

xgreenx reviewed Feb 6, 2025

View reviewed changes

netrome mentioned this pull request Feb 6, 2025

fix: Remove dangerous array lookup #2677

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fault proving: Populate ContractsRawCode when processing Create Transaction in global merkle root crate #2652

Fault proving: Populate ContractsRawCode when processing Create Transaction in global merkle root crate #2652

acerone85 commented Jan 30, 2025

netrome left a comment

xgreenx Jan 31, 2025

acerone85 Feb 3, 2025

netrome left a comment

MitchTurner Feb 4, 2025

acerone85 Feb 5, 2025

MitchTurner Feb 4, 2025

acerone85 Feb 5, 2025

MitchTurner Feb 4, 2025

acerone85 Feb 5, 2025

xgreenx Feb 6, 2025

netrome Feb 6, 2025

netrome Feb 6, 2025

		// CreateBody is not publicly exported, so we need to use a generic type
		// and ensure that all trait bounds are satisfied

Fault proving: Populate ContractsRawCode when processing Create Transaction in global merkle root crate #2652

Fault proving: Populate ContractsRawCode when processing Create Transaction in global merkle root crate #2652

Conversation

acerone85 commented Jan 30, 2025

Linked Issues/PRs

Description

Checklist

Before requesting review

After merging, notify other teams

netrome left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netrome left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment