Added a new Upgrade transaction to perform a network upgrade
#567
Conversation
| @@ -50,6 +51,8 @@ Sway | |||
| TAI | |||
| TODO | |||
| TypeScript | |||
| UpgradePurpose | |||
There was a problem hiding this comment.
Instead of adding UpgradePurpose and UpgradePurposeType to this file, you can just wrap them in backticks like UpgradePurpose so the spell checker will ignore them. Otherwise LGTM!
src/protocol/tx-validity.md
Outdated
| @@ -179,6 +179,12 @@ def metadata_gas_fees(tx) -> int: | |||
| elif tx.type == TransactionType.Script: | |||
| # add intrinsic cost of calculating the transaction id | |||
| total += sha256_gas_fee(size(tx)) | |||
| elif tx.type == TransactionType.Upgrade: | |||
| # add intrinsic cost of calculating the transaction id | |||
| total += sha256_gas_fee(size(tx)) | |||
There was a problem hiding this comment.
Isn't this same for all tx types? Could we maybe move it outside the if statement, e.g. to the initializer of total? Also above we add it for every created output contract which doesn't seem correct. Not really part of this PR, but noticed that while reviewing.
| - Any input uses non-base asset. | ||
| - Any output is of type `OutputType.Contract` or `OutputType.Variable` or `OutputType.Message` | ||
| - More than one output is of type `OutputType.Change` with `asset_id` of zero | ||
| - Any output is of type `OutputType.Change` with non-zero `asset_id` | ||
| - Any output is of type `OutputType.Change` with non-base `asset_id` |
There was a problem hiding this comment.
nit: Out-of-scope for this PR. Still nice fixes and having them here is ok for me.
src/tx-format/transaction.md
Outdated
| Given helper `max_gas()` returns the maximum gas that the transaction can use. | ||
| Given helper `count_ones()` that returns the number of ones in the binary representation of a field. | ||
| Given helper `count_variants()` that returns the number of variants in an enum. | ||
| Given helper `sum_variants()` that sums all variants of an enum. |
There was a problem hiding this comment.
We're repeating these quite a bit. Maybe we should have a centralized list of these? Although I think these might be almost evident enough tha they don't need explanations.
src/tx-format/upgrade_purpose.md
Outdated
|
|
||
| Transaction is invalid if: | ||
|
|
||
| - `type > UpgradePurposeType.StateTransition` |
There was a problem hiding this comment.
nit: I'd prefer to just say that "type is not valid UpgradePurposeType value"
spell-check-custom-words.txt
Outdated
| UpgradePurpose | ||
| UpgradePurposeType |
There was a problem hiding this comment.
nit: these should probably just be in backticks (code) everywhere, so we don't need to spellcheck-allow them
| @@ -174,9 +174,12 @@ def metadata_gas_fees(tx) -> int: | |||
| # add intrinsic cost of calculating the contract id | |||
| # size = 4 byte seed + 32 byte salt + 32 byte code root + 32 byte state root | |||
| total += sha256_gas_fee(100) | |||
| # add intrinsic cost of calculating the transaction id | |||
| total += sha256_gas_fee(size(tx)) | |||
| elif tx.type == TransactionType.Script: | |||
There was a problem hiding this comment.
Wait, what happened to the Script path?
There was a problem hiding this comment.
It is handled below=)
Since that part is identical for all transactions except Mint
Corresponding implementation: FuelLabs/fuel-vm#707
The change adds a new
Upgradetransaction that allows upgrading either consensus parameters or state transition function used by the network to produce future blocks.The purpose of the upgrade is defined by the
Upgrade Purposetype.The
Upgradetransaction is chargeable, and the sender should pay for it. Transaction inputs should contain only base assets.Only the privileged address can upgrade the network. The privileged address can be either a real account or a predicate.
We use postcard algorithm to serialize and deserialize consensus parameters during the upgrade. This algorithm works with numbers in a more sufficient way and compresses them well.