feat: tx weight takes tariscript and output features into account [igor] #3411
Conversation
This file contains 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
sdbondi
force-pushed
the
core-output-features-canonical-encoding
branch
3 times, most recently
from
9c2302a to
736f953
Compare
sdbondi
force-pushed
the
core-output-features-canonical-encoding
branch
from
736f953 to
729f88f
Compare
| let mut buf = Vec::new(); | ||
| bincode::serialize_into(&mut buf, self).unwrap(); // this should not fail | ||
| /// Encodes output features for a v1 block | ||
| pub fn to_v1_bytes(&self) -> Vec<u8> { |
There was a problem hiding this comment.
I think we should add a version to the output or perhaps OutputFeatures so that we know which version was used in the hash etc.
There was a problem hiding this comment.
Yeah think that makes sense - will do - I'll modify the hashes depending on the output version
There was a problem hiding this comment.
The downside is additional storage and it turns this PR into a breaking one, blockchains will need to be resynced so not too bad
There was a problem hiding this comment.
The default impl for OutputFeatures no longer makes sense as the version depends on your network -> block version, which entails lots of changes.
The intention was to remove the v1/v2 concept and just use consensus encoding at a convenient point (like a testnet reset). But it may. be worth putting in the time to include "transaction/UTXO versioning" while maintaining backward-compatibility. Perhaps for another PR since this one is currently non-breaking for weatherwax?
sdbondi
force-pushed
the
core-output-features-canonical-encoding
branch
12 times, most recently
from
74b7468 to
621f8bc
Compare
- adds consensus encoding for tari script and output features - igor: tx weight takes TariScript and OutputFeatures metadata into account as per tari-project#3368 - igor: weights updated as per tari-project#3368 - add `TransactionWeight` parameters for weatherwax and igor networks and set them in consensus constants - set max block weight for igor (+95% adjust to allow similar number of max inputs/outputs/kernels ~2Mb) - decouple chain_storage from consensus_contstants module to allow wallet to use it - minor simlification of wallet coin split logic - wallet and base node tests updated as needed
sdbondi
force-pushed
the
core-output-features-canonical-encoding
branch
from
621f8bc to
b43270e
Compare
* development: fix: fix confusing names in get_balance functions (tari-project#3447) feat: add sql query to obtain balance (tari-project#3446) fix: u64->i64->u64 conversion; chain split height as u64 (tari-project#3442)
* development: fix: add display_currency_decimal method (tari-project#3445) fix: add sanity checks to prepare_new_block (tari-project#3448) test: profile wallet sqlite database operations (tari-project#3451) test: create cucumber test directory if necessary (tari-project#3452) chore: improve cucumber tags and run time speed (tari-project#3439)
stringhandler
previously approved these changes
Oct 15, 2021
sdbondi
force-pushed
the
core-output-features-canonical-encoding
branch
from
32cfc93 to
3cf6b47
Compare
stringhandler
previously approved these changes
Oct 18, 2021
* development: fix: remove unnecessary wallet dependency (tari-project#3438) test: simplify cucumber tests (tari-project#3457)
* development: fix: ensure that accumulated orphan chain data is committed before header validation (tari-project#3462) fix: remove is_synced check for transaction validation (tari-project#3459) feat: improve logging for tari_mining_node (tari-project#3449)
sdbondi
force-pushed
the
core-output-features-canonical-encoding
branch
from
bc2c9e7 to
1c208da
Compare
stringhandler
approved these changes
Oct 19, 2021
sdbondi
added a commit
to sdbondi/tari
that referenced
this pull request
Oct 19, 2021
* development: feat: tx weight takes tariscript and output features into account [igor] (tari-project#3411)
aviator-app bot
pushed a commit
that referenced
this pull request
Oct 19, 2021
Description --- Remove consensus breaking change from #3411 Motivation and Context --- Breaks consensus on weatherwax How Has This Been Tested? --- Synced base node
sdbondi
added a commit
to sdbondi/tari
that referenced
this pull request
Oct 25, 2021
* development: (31 commits) feat!: revalidate all outputs (tari-project#3471) fix: check SAF message inflight and check stored_at is in past (tari-project#3444) feat!: apps should not depend on other app configs (tari-project#3469) fix: fix recovery test reporting message (tari-project#3479) chore: improve cucumber tests to wait for broadcast (tari-project#3461) test: use TCP node for daily sync test (tari-project#3464) fix: remove unbounded vec allocations from base node grpc/p2p messaging (tari-project#3467) fix: upgrade rustyline dependencies (tari-project#3476) fix(dht): discard encrypted message with no destination (tari-project#3472) fix: remove consensus breaking change in transaction input (tari-project#3474) feat: tx weight takes tariscript and output features into account [igor] (tari-project#3411) fix: validate dht header before dedup cache (tari-project#3468) fix: sha256sum isn't available on all *nix platforms (tari-project#3466) fix: typo in console wallet (tari-project#3465) fix: ensure that accumulated orphan chain data is committed before header validation (tari-project#3462) fix: remove is_synced check for transaction validation (tari-project#3459) feat: improve logging for tari_mining_node (tari-project#3449) fix: remove unnecessary wallet dependency (tari-project#3438) test: simplify cucumber tests (tari-project#3457) ci: create script to update DNS records from hashes.txt (tari-project#3458) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
account as per docs: add block/transaction weights to RFC #3368
TransactionWeightparameters forweatherwaxandigornetworksand set them in consensus constants
igor(+95% adjust to allow similar number ofmax inputs/outputs/kernels ~2Mb)
consensus_contstantsmodule to allowwallet to use it
Motivation and Context
See #3368
How Has This Been Tested?
Existing tests updated, manually checked normal and one-sided transaction sending between console wallets/mobile on watherwax and igor