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

Tracking: Malleable inputs for Checked<Transaction> type #651

Closed
xgreenx opened this issue Jan 2, 2024 · 7 comments · Fixed by #678
Closed

Tracking: Malleable inputs for Checked<Transaction> type #651

xgreenx opened this issue Jan 2, 2024 · 7 comments · Fixed by #678
Assignees

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Jan 2, 2024

Maybe move fuel_core_executor::Executor::compute_inputs to Checked or add a way of updating it.

image
@xgreenx
Copy link
Collaborator Author

xgreenx commented Jan 22, 2024

Actually not, we decided to keep them malleable FuelLabs/fuel-specs#542. But we still need to update the implementation of FuelVM to zero fields

@Dentosal
Copy link
Member

@xgreenx Do I understand correctly that the issue description is now outdated, and we just have to zero the malleable fields before execution?

@xgreenx
Copy link
Collaborator Author

xgreenx commented Feb 13, 2024

Yep=) We need to update the specification to reflect this change and update our codebase to zero all malleable fields(except witnesses)

@Dentosal
Copy link
Member

Yep=) We need to update the specification to reflect this change and update our codebase to zero all malleable fields(except witnesses)

So like this? FuelLabs/fuel-specs#545 It feels weird to even have things like InputCoin::tx_pointer if that's zero-initialized.

@xgreenx
Copy link
Collaborator Author

xgreenx commented Feb 13, 2024

Yeah, looks correct. Maybe it also worth removing the corresponding GTF

All instances of TxPointer exist only for light clients and for fraud-proofing purposes. It simplifies the flow significantly allowing the passing of direct data based on the pointer=)

@Dentosal
Copy link
Member

@xgreenx Currently, receipts_root is updates in-memory after modifying receipts. Moreover, it's specified to be:

Note: when executing a script, receiptsRoot is initialized to zero.

The receipts root receiptsRoot is the root of the binary Merkle tree of receipts. If there are no receipts, its value is set to the root of the empty tree, i.e. 0xe3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.

This seems to conflict itself. What's the actual behavior we want here? Do we even need access to this value during execution?

@xgreenx
Copy link
Collaborator Author

xgreenx commented Feb 16, 2024

Note: when executing a script, receiptsRoot is initialized to zero.

By this phrase, we usually mean at the start of the execution of the transaction something should be zero=)

So, modifying it shouldn't affect the validity of the block(I mean the validator should receive the same result as a producer).

But, I don't think there is any use case for accessing the receipts root during the execution. Maybe somehow for the old fraud-proof model, but since we still need access to all old receipts to calculate the next receipt root, I don't think so. cc @Voxelot @dmihal

Maybe it makes sense from a performance perspective only to do one root calculation at the end of the script execution. But it means we need to update the specification since it says that we need to update it each time:

Append a receipt to the list of receipts, modifying tx.receiptsRoot:

@Dentosal Dentosal changed the title Allows updating zeroed fields of inputs for Checked<Transaction> type Tracking: Malleable inputs for Checked<Transaction> type Feb 22, 2024
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 a pull request may close this issue.

2 participants