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

A new Upload transaction to upload the huge bytecode on the chain #720

Merged
merged 72 commits into from
Apr 12, 2024

Conversation

xgreenx
Copy link
Collaborator

@xgreenx xgreenx commented Apr 10, 2024

Related FuelLabs/fuel-core#1754 and FuelLabs/fuel-specs#570

The change adds a new Upload transaction that allows uploading huge byte code on chain subsection by subsection.

The Upload transaction is chargeable and is twice as expensive as the Create transaction. Anyone can submit this transaction.

Before requesting review

  • I have reviewed the code myself

xgreenx and others added 30 commits March 21, 2024 07:37
Reduced default `MAX_SIZE` to be 110kb.
Reduced default `MAX_CONTRACT_SIZE` to be 100kb.
# Conflicts:
#	CHANGELOG.md
#	fuel-tx/src/builder.rs
#	fuel-tx/src/tests/valid_cases/transaction.rs
#	fuel-tx/src/transaction/consensus_parameters.rs
#	fuel-vm/src/checked_transaction.rs
#	fuel-vm/src/tests/limits.rs
#	fuel-vm/src/tests/validation.rs
…ction

# Conflicts:
#	fuel-tx/src/transaction/consensus_parameters.rs
# Conflicts:
#	fuel-tx/src/transaction/consensus_parameters.rs
#	fuel-vm/src/tests/validation.rs
…ansaction

# Conflicts:
#	fuel-tx/src/transaction/types/script.rs
#	fuel-vm/src/interpreter.rs
xgreenx and others added 6 commits April 9, 2024 21:24
Co-authored-by: Mitchell Turner <james.mitchell.turner@gmail.com>
Co-authored-by: Mitchell Turner <james.mitchell.turner@gmail.com>
@xgreenx xgreenx self-assigned this Apr 10, 2024
@xgreenx xgreenx requested a review from a team April 10, 2024 21:51
@xgreenx xgreenx changed the title Feature/upload transaction A new Upload transaction to upload the huge bytecode on the chain Apr 10, 2024
@xgreenx xgreenx added the breaking A breaking api change label Apr 11, 2024
Base automatically changed from feature/process-upgrade-transaction-within-interpreter to master April 11, 2024 18:54
@xgreenx xgreenx requested a review from Dentosal April 12, 2024 08:09
Copy link
Member

@MitchTurner MitchTurner left a comment

Choose a reason for hiding this comment

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

Tangentially related:

Something I've said before, and I still think would be good is instead of Upload and Script and Create, etc, we should have our transactions named UploadTx and ScriptTx and CreateTx, etc. I know the context is in the namespace, but it is never with it's namespace, so unless you understand the context well, you have to make couple clicks before you understand what they are.

Event then, it's far from obvious that Script would be a type of transaction, to me Script suggests that it is the code run by smart contracts, and never what you'd use for a simple transfer (but it is what you'd use). So even ScriptTx probably isn't clear enough.

fuel-tx/src/tests/bytes.rs Outdated Show resolved Hide resolved
@@ -32,6 +32,15 @@ fn to_from_str() {

assert_eq!(tx, tx_p);
});
TransactionFactory::<_, Upload>::from_seed(1295)
Copy link
Member

Choose a reason for hiding this comment

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

I was gonna suggest maybe drying this up a little with a shared function:

fn transaction_round_trip_serde<T>()
    where 
        TransactionFactory<StdRng, T>: Iterator,
        T: Into<Transaction>
     {
    TransactionFactory::<_, Upgrade>::from_seed(1295)
        .take(20)
        .for_each(|(tx, _)| {
            // given
            let tx: Transaction = tx.into();
            // when
            let tx_p = tx.to_json();
            let tx_p = Transaction::from_json(tx_p).expect("failed to restore tx");
            // then
            assert_eq!(tx, tx_p);
}

I don't know if this is much better with all the generics.

I think the name to_from_str could be a little clearer either way though.

fuel-tx/src/tests/valid_cases/transaction/upload.rs Outdated Show resolved Hide resolved
fuel-tx/src/tests/valid_cases/transaction/upload.rs Outdated Show resolved Hide resolved
fuel-tx/src/transaction.rs Outdated Show resolved Hide resolved
@xgreenx xgreenx requested review from MitchTurner and a team April 12, 2024 21:51
@xgreenx xgreenx added this pull request to the merge queue Apr 12, 2024
Merged via the queue into master with commit 747532e Apr 12, 2024
38 checks passed
@xgreenx xgreenx deleted the feature/upload-transaction branch April 12, 2024 22:58
@xgreenx xgreenx mentioned this pull request Apr 15, 2024
xgreenx added a commit to FuelLabs/fuel-specs that referenced this pull request Apr 15, 2024
…570)

Corresponding implementation:
FuelLabs/fuel-vm#720
The change adds a new `Upload` transaction that allows uploading huge
byte code on chain subsection by subsection.

The `Upload` transaction is chargeable and is twice as expensive as the
`Create` transaction. Anyone can submit this transaction.

The specification contains more description about the flow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking api change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants