feat(transactions): add transaction weight for ValueTransferTransacti… #1301
Conversation
There was a problem hiding this comment.
Nothing to say apart from what @tmpolaczyk and @girazoki commented! Great PR!
| let vt_body = | ||
| VTTransactionBody::new(vec![Input::default()], vec![ValueTransferOutput::default()]); | ||
| let vt_tx = VTTransaction::new(vt_body, vec![KeyedSignature::default()]); | ||
| assert_eq!(INPUT_SIZE + OUTPUT_SIZE * GAMMA, vt_tx.weight()); |
There was a problem hiding this comment.
I sort of mistrust tests that re-implement the logic of the computations. I can't see how one piece of code would not conform with another that has been written by the same person using the same programming language.
Moreover, I think that any breaking change in ValueTransferTransaction::weight would go unnoticed — these tests would simply pass.
Using static numbers here is also more useful IMO because they can be used as test vectors for other implementations.
There was a problem hiding this comment.
We can have both, simply add a line checking the actual value.
lrubiorod
force-pushed
the
tx_weight
branch
4 times, most recently
from
77e8c75
to
965acb6
Compare
data_structures/src/error.rs
Outdated
| display = "Data Request Transactions weight: {}, exceed the maximum weight: {}", | ||
| weight, max_weight | ||
| )] | ||
| DataRequestWeightOverflow { weight: u32, max_weight: u32 }, |
There was a problem hiding this comment.
Suggestions from previous block also apply here.
data_structures/src/transaction.rs
Outdated
| let inputs_len = u32::try_from(self.body.inputs.len()).unwrap_or(u32::MAX); | ||
| let outputs_len = u32::try_from(self.body.outputs.len()).unwrap_or(u32::MAX); | ||
| let inputs_weight = inputs_len.saturating_mul(INPUT_SIZE); | ||
| let outputs_weight = outputs_len.saturating_mul(OUTPUT_SIZE); |
There was a problem hiding this comment.
This will need to be changed after #1306, right?
There was a problem hiding this comment.
yes, later it would be only OUTPUT_SIZE
| } | ||
|
|
||
| if new_block_weight == max_block_weight { | ||
| if new_vt_weight == max_vt_weight { |
There was a problem hiding this comment.
If we know the weight of the lighter possible VTT, we can add that to the left operand in the equality and save a crazy amount of iterations here.
| dr_weight += transaction_weight; | ||
| } | ||
|
|
||
| if new_dr_weight == max_dr_weight { |
data_structures/src/chain.rs
Outdated
| pub fn weight(&self) -> u32 { | ||
| // Time lock: 8 bytes | ||
|
|
||
| let mut retrieves_weight: u32 = 0; |
There was a problem hiding this comment.
| let mut retrieves_weight: u32 = 0; | |
| let mut retrievals_weight: u32 = 0; |
data_structures/src/error.rs
Outdated
| ValueTransferWeightLimitExceeded { weight: u32, max_weight: u32 }, | ||
| /// Data Request weight limit exceeded | ||
| #[fail( | ||
| display = "Data Request Transactions weight: {}, exceed the maximum weight: {}", |
There was a problem hiding this comment.
| display = "Data Request Transactions weight: {}, exceed the maximum weight: {}", | |
| display = "Total weight of Data Request Transactions in a block ({}) exceeds the limit ({})", |
lrubiorod
force-pushed
the
tx_weight
branch
2 times, most recently
from
f080652
to
8b4140a
Compare
…ons and DRTransactions
…ons and DRTransactions
Close #1298
Close #1299