Skip to content

Commit

Permalink
Remove recovery byte
Browse files Browse the repository at this point in the history
Removed recovery byte from the output features due to the recently added
encrypted value to a transaction output being able to fulfil the original
intent of the recovery byte, i.e. to let a wallet pinpoint the UTXO's it
owns.
  • Loading branch information
hansieodendaal committed Jul 14, 2022
1 parent b14374d commit 4e400e8
Show file tree
Hide file tree
Showing 45 changed files with 4,169 additions and 4,762 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -217,12 +217,12 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: test
args: --no-run --locked --all-features
args: --no-run --locked --all-features --release
- name: cargo test
uses: actions-rs/cargo@v1
with:
command: test
args: -v --all-features
args: -v --all-features --release
# Allows other workflows to know the PR number
artifacts:
name: test
Expand Down
3 changes: 0 additions & 3 deletions applications/tari_app_grpc/proto/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,6 @@ message OutputFeatures {
// require a min maturity of the Coinbase_lock_height, this should be checked on receiving new blocks.
uint64 maturity = 3;
bytes metadata = 4;
// The recovery byte - not consensus critical - can help reduce the bandwidth with wallet recovery or in other
// instances when a wallet needs to request the complete UTXO set from a base node.
uint32 recovery_byte = 5;
SideChainFeatures sidechain_features = 6;
bytes unique_id = 7;

Expand Down
2 changes: 0 additions & 2 deletions applications/tari_app_grpc/src/conversions/output_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ impl TryFrom<grpc::OutputFeatures> for OutputFeatures {
)?,
OutputType::from_byte(output_type).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
features.maturity,
u8::try_from(features.recovery_byte).map_err(|_| "Invalid recovery byte: overflowed u8")?,
features.metadata,
unique_id,
sidechain_features,
Expand All @@ -92,7 +91,6 @@ impl From<OutputFeatures> for grpc::OutputFeatures {
maturity: features.maturity,
metadata: features.metadata,
unique_id: features.unique_id.unwrap_or_default(),
recovery_byte: u32::from(features.recovery_byte),
sidechain_features: features.sidechain_features.map(|v| *v).map(Into::into),

// TODO: Deprecated
Expand Down
1 change: 0 additions & 1 deletion applications/tari_explorer/partials/OutputFeatures.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ Contract constitution change acceptance
</dd>
<dt>Maturity</dt><dd>{{maturity}}</dd>
{{#if metadata.length}}<dt>Metadata</dt><dd>{{json metadata}}</dd>{{/if}}
<dt>Recovery byte</dt><dd>{{recovery_byte}}</dd>
{{#if sidechain_features}}
<dt>Sidechain Features</dt>
{{>SideChainFeatures sidechain_features}}
Expand Down
8,000 changes: 4,000 additions & 4,000 deletions base_layer/core/src/blocks/faucets/dibbler_faucet.json

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion base_layer/core/src/blocks/genesis_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,6 @@ fn get_dibbler_genesis_block_raw() -> Block {
version: OutputFeaturesVersion::get_current_version(),
output_type: OutputType::Coinbase,
maturity: 60,
recovery_byte: 0,
metadata: Vec::new(),
unique_id: None,
sidechain_features: None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ mod fetch_utxo_by_unique_id {
// Height 1
let (blocks, outputs) = add_many_chained_blocks(1, &db);

let mut features = OutputFeatures {
let features = OutputFeatures {
output_type: OutputType::MintNonFungible,
parent_public_key: Some(asset_pk.clone()),
unique_id: Some(unique_id.clone()),
Expand All @@ -746,9 +746,8 @@ mod fetch_utxo_by_unique_id {
to: vec![500 * T],
fee: 5.into(),
lock: 0,
features: features.clone()
features: features
)]);
features.set_recovery_byte(tx_outputs[0].features.recovery_byte);

let asset_utxo1 = tx_outputs.iter().find(|o| o.features == features).unwrap();

Expand All @@ -763,7 +762,6 @@ mod fetch_utxo_by_unique_id {
.fetch_utxo_by_unique_id(Some(asset_pk.clone()), unique_id.clone(), None)
.unwrap()
.unwrap();
features.set_recovery_byte(info.output.as_transaction_output().unwrap().features.recovery_byte);
assert_eq!(info.output.as_transaction_output().unwrap().features, features);
let expected_commitment =
CommitmentFactory::default().commit_value(&asset_utxo1.spending_key, asset_utxo1.value.as_u64());
Expand All @@ -772,7 +770,7 @@ mod fetch_utxo_by_unique_id {
expected_commitment
);

let mut features = OutputFeatures {
let features = OutputFeatures {
parent_public_key: Some(asset_pk.clone()),
unique_id: Some(unique_id.clone()),
..Default::default()
Expand All @@ -784,7 +782,6 @@ mod fetch_utxo_by_unique_id {
lock: 0,
features: features
)]);
features.set_recovery_byte(tx_outputs[0].features.recovery_byte);

let asset_utxo2 = tx_outputs.iter().find(|o| o.features == features).unwrap();

Expand Down
3 changes: 1 addition & 2 deletions base_layer/core/src/covenants/fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ mod test {

#[test]
fn it_retrieves_the_value_as_ref() {
let mut features = OutputFeatures {
let features = OutputFeatures {
maturity: 42,
..Default::default()
};
Expand All @@ -616,7 +616,6 @@ mod test {
})
.pop()
.unwrap();
features.set_recovery_byte(output.features.recovery_byte);
let r = OutputField::Features.get_field_value_ref::<OutputFeatures>(&output);
assert_eq!(*r.unwrap(), features);
}
Expand Down
3 changes: 0 additions & 3 deletions base_layer/core/src/proto/transaction.proto
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,6 @@ message OutputFeatures {
// require a min maturity of the Coinbase_lock_height, this should be checked on receiving new blocks.
uint64 maturity = 3;
bytes metadata = 4;
// The recovery byte - not consensus critical - can help reduce the bandwidth with wallet recovery or in other
// instances when a wallet needs to request the complete UTXO set from a base node.
uint32 recovery_byte = 5;
SideChainFeatures sidechain_features = 6;


Expand Down
2 changes: 0 additions & 2 deletions base_layer/core/src/proto/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ impl TryFrom<proto::types::OutputFeatures> for OutputFeatures {
)?,
OutputType::from_byte(flags).ok_or_else(|| "Invalid or unrecognised output type".to_string())?,
features.maturity,
u8::try_from(features.recovery_byte).map_err(|_| "Invalid recovery byte: overflowed u8")?,
features.metadata,
unique_id,
sidechain_features,
Expand Down Expand Up @@ -357,7 +356,6 @@ impl From<OutputFeatures> for proto::types::OutputFeatures {
sidechain_checkpoint: features.sidechain_checkpoint.map(|s| s.into()),
version: features.version as u32,
committee_definition: features.committee_definition.map(|c| c.into()),
recovery_byte: u32::from(features.recovery_byte),
sidechain_features: features.sidechain_features.map(|v| *v).map(Into::into),
}
}
Expand Down
4 changes: 1 addition & 3 deletions base_layer/core/src/transactions/coinbase_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,7 @@ impl CoinbaseBuilder {
.factories
.commitment
.commit_value(&spending_key, total_reward.as_u64());
let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, self.rewind_data.as_ref());
let output_features = OutputFeatures::create_coinbase(height + constants.coinbase_lock_height(), recovery_byte);
let output_features = OutputFeatures::create_coinbase(height + constants.coinbase_lock_height());
let excess = self.factories.commitment.commit_value(&spending_key, 0);
let kernel_features = KernelFeatures::create_coinbase();
let metadata = TransactionMetadata::default();
Expand Down Expand Up @@ -374,7 +373,6 @@ mod test {

let rewind_data = RewindData {
rewind_blinding_key: rewind_blinding_key.clone(),
recovery_byte_key: PrivateKey::random(&mut OsRng),
encryption_key: PrivateKey::random(&mut OsRng),
};

Expand Down
74 changes: 7 additions & 67 deletions base_layer/core/src/transactions/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,7 @@ impl Default for UtxoTestParams {
Self {
value: 10.into(),
script: script![Nop],
features: OutputFeatures {
recovery_byte: u8::MAX,
..Default::default()
},
features: OutputFeatures::default(),
input_data: None,
covenant: Covenant::default(),
output_version: None,
Expand Down Expand Up @@ -153,7 +150,6 @@ impl TestParams {
transaction_weight: TransactionWeight::v2(),
rewind_data: RewindData {
rewind_blinding_key: PrivateKey::random(&mut OsRng),
recovery_byte_key: PrivateKey::random(&mut OsRng),
encryption_key: PrivateKey::random(&mut OsRng),
},
}
Expand All @@ -175,8 +171,6 @@ impl TestParams {
let commitment = self
.commitment_factory
.commit_value(&self.spend_key, params.value.as_u64());
let updated_features =
OutputFeatures::features_with_updated_recovery_byte(&commitment, rewind_data, &params.features);

let encrypted_value = if let Some(rewind_data) = rewind_data {
EncryptedValue::encrypt_value(&rewind_data.encryption_key, &commitment, params.value).unwrap()
Expand All @@ -188,7 +182,7 @@ impl TestParams {
params.value,
&self.spend_key,
&params.script,
&updated_features,
&params.features,
&self.sender_offset_private_key,
&params.covenant,
&encrypted_value,
Expand All @@ -201,7 +195,7 @@ impl TestParams {
.unwrap_or_else(TransactionOutputVersion::get_current_version),
params.value,
self.spend_key.clone(),
updated_features,
params.features,
params.script.clone(),
params
.input_data
Expand All @@ -215,38 +209,6 @@ impl TestParams {
)
}

pub fn update_unblinded_output_with_updated_output_features(
&self,
uo: UnblindedOutput,
updated_features: OutputFeatures,
) -> UnblindedOutput {
let metadata_signature = TransactionOutput::create_final_metadata_signature(
TransactionOutputVersion::get_current_version(),
uo.value,
&uo.spending_key,
&uo.script,
&updated_features,
&self.sender_offset_private_key,
&uo.covenant,
&uo.encrypted_value,
)
.unwrap();

UnblindedOutput::new_current_version(
uo.value,
uo.spending_key.clone(),
updated_features,
uo.script,
uo.input_data.clone(),
uo.script_private_key.clone(),
uo.sender_offset_public_key.clone(),
metadata_signature,
uo.script_lock_height,
uo.covenant,
uo.encrypted_value,
)
}

pub fn get_script_public_key(&self) -> PublicKey {
PublicKey::from_secret_key(&self.script_private_key)
}
Expand Down Expand Up @@ -331,7 +293,7 @@ pub fn create_unblinded_coinbase(test_params: &TestParams, height: u64) -> Unbli
let constants = rules.consensus_constants(height);
test_params.create_unblinded_output(UtxoTestParams {
value: rules.get_block_reward_at(height),
features: OutputFeatures::create_coinbase(height + constants.coinbase_lock_height(), 0x00),
features: OutputFeatures::create_coinbase(height + constants.coinbase_lock_height()),
..Default::default()
})
}
Expand Down Expand Up @@ -364,14 +326,6 @@ pub fn create_unblinded_output_with_rewind_data(
})
}

pub fn update_unblinded_output_with_updated_output_features(
test_params: &TestParams,
uo: UnblindedOutput,
updated_features: OutputFeatures,
) -> UnblindedOutput {
test_params.update_unblinded_output_with_updated_output_features(uo, updated_features)
}

/// The tx macro is a convenience wrapper around the [create_tx] function, making the arguments optional and explicit
/// via keywords.
#[macro_export]
Expand Down Expand Up @@ -719,11 +673,6 @@ pub fn create_stx_protocol(schema: TransactionSchema) -> (SenderTransactionProto
}
for mut utxo in schema.to_outputs {
let test_params = TestParams::new();
let commitment = factories
.commitment
.commit_value(&utxo.spending_key, utxo.value.as_u64());
let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, None);
utxo.features.set_recovery_byte(recovery_byte);
utxo.metadata_signature = TransactionOutput::create_final_metadata_signature(
output_version,
utxo.value,
Expand All @@ -749,14 +698,7 @@ pub fn create_stx_protocol(schema: TransactionSchema) -> (SenderTransactionProto

let script = script!(Nop);
let covenant = Covenant::default();
let commitment = factories
.commitment
.commit_value(&test_params_change_and_txn.change_spend_key, change.as_u64());
let recovery_byte = OutputFeatures::create_unique_recovery_byte(&commitment, None);
let change_features = OutputFeatures {
recovery_byte,
..Default::default()
};
let change_features = OutputFeatures::default();

let encrypted_value = EncryptedValue::default();

Expand Down Expand Up @@ -827,22 +769,20 @@ pub fn create_utxo(
let commitment = factories.commitment.commit_value(&keys.k, value.into());
let proof = factories.range_proof.construct_proof(&keys.k, value.into()).unwrap();

let updated_features = OutputFeatures::features_with_updated_recovery_byte(&commitment, None, features);

let metadata_sig = TransactionOutput::create_final_metadata_signature(
TransactionOutputVersion::get_current_version(),
value,
&keys.k,
script,
&updated_features,
features,
&offset_keys.k,
covenant,
&EncryptedValue::default(),
)
.unwrap();

let utxo = TransactionOutput::new_current_version(
updated_features,
features.clone(),
commitment,
proof.into(),
script.clone(),
Expand Down
Loading

0 comments on commit 4e400e8

Please sign in to comment.