-
Notifications
You must be signed in to change notification settings - Fork 7
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
360 support confirm scp messages in the stellar relay when proving transaction validity #362
Merged
ebma
merged 15 commits into
main
from
360-support-confirm-scp-messages-in-the-stellar-relay-when-proving-transaction-validity
Jul 4, 2023
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
616f6e1
Support 'Confirm' SCP messages in stellar-relay
ebma 52d073f
Amend
ebma 448a302
Rename error
ebma cd8e715
Use correct n_h parameter
ebma 52be5b3
Small renaming
ebma fd4155d
Fix compilation errors
ebma d64222d
Fix bug in `get_tx_set_hash()` function
ebma 2f9157d
Add test for slot_index
ebma ba33541
Change test helper functions to support confirmed or externalized mes…
ebma 3fbef1a
Add more test functions
ebma f9847f3
Add test for mismatching n_h values
ebma ab33492
Update frame-weight-template.hbs
ebma 4011e2b
Regenerate weights
ebma 2ac8439
Simplify externalized value check
ebma de66247
Merge branch 'main' into 360-support-confirm-scp-messages-in-the-stel…
ebma File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,10 +37,7 @@ pub mod pallet { | |
use substrate_stellar_sdk::{ | ||
compound_types::UnlimitedVarArray, | ||
network::{Network, PUBLIC_NETWORK, TEST_NETWORK}, | ||
types::{ | ||
NodeId, ScpEnvelope, ScpStatementExternalize, ScpStatementPledges, StellarValue, | ||
TransactionSet, | ||
}, | ||
types::{NodeId, ScpEnvelope, ScpStatementPledges, StellarValue, TransactionSet, Value}, | ||
Hash, TransactionEnvelope, XdrCodec, | ||
}; | ||
|
||
|
@@ -102,27 +99,34 @@ pub mod pallet { | |
pub enum Error<T> { | ||
Base64DecodeError, | ||
BoundedVecCreationFailed, | ||
DuplicateOrganizationId, | ||
DuplicateValidatorPublicKey, | ||
EmptyEnvelopeSet, | ||
EnvelopeSignedByUnknownValidator, | ||
EnvelopeSlotIndexMismatch, | ||
ExternalizedNHMismatch, | ||
ExternalizedValueMismatch, | ||
ExternalizedValueNotFound, | ||
FailedToComputeNonGenericTxSetContentHash, | ||
InvalidEnvelopeSignature, | ||
InvalidQuorumSetNotEnoughOrganizations, | ||
InvalidQuorumSetNotEnoughValidators, | ||
InvalidScpPledge, | ||
InvalidEnvelopeSignature, | ||
InvalidXDR, | ||
NoOrganizationsRegisteredForNetwork, | ||
NoValidatorsRegisteredForNetwork, | ||
InvalidTransactionSet, | ||
InvalidTransactionXDR, | ||
InvalidXDR, | ||
MissingExternalizedMessage, | ||
NoOrganizationsRegistered, | ||
NoOrganizationsRegisteredForNetwork, | ||
NoValidatorsRegistered, | ||
NoValidatorsRegisteredForNetwork, | ||
OrganizationLimitExceeded, | ||
SlotIndexIsNone, | ||
TransactionMemoDoesNotMatch, | ||
TransactionNotInTransactionSet, | ||
TransactionSetHashCreationFailed, | ||
TransactionSetHashMismatch, | ||
ValidatorLimitExceeded, | ||
DuplicateOrganizationId, | ||
DuplicateValidatorPublicKey, | ||
FailedToComputenonGenericTxSetContentHash, | ||
} | ||
|
||
#[pallet::storage] | ||
|
@@ -557,6 +561,21 @@ pub mod pallet { | |
// Make sure that at least one validator is registered | ||
ensure!(!validators.is_empty(), Error::<T>::NoValidatorsRegistered); | ||
|
||
// Make sure that the envelope set is not empty | ||
ensure!(!envelopes.len() > 0, Error::<T>::EmptyEnvelopeSet); | ||
|
||
let externalized_envelope = envelopes | ||
.get_vec() | ||
.iter() | ||
.find(|envelope| match envelope.statement.pledges { | ||
ScpStatementPledges::ScpStExternalize(_) => true, | ||
_ => false, | ||
}) | ||
.ok_or(Error::<T>::MissingExternalizedMessage)?; | ||
|
||
// Variable used to check if all envelopes are using the same slot index | ||
let slot_index: u64 = externalized_envelope.statement.slot_index; | ||
|
||
for envelope in envelopes.get_vec() { | ||
let node_id = envelope.statement.node_id.clone(); | ||
let node_id_found = validators | ||
|
@@ -565,25 +584,51 @@ pub mod pallet { | |
|
||
ensure!(node_id_found, Error::<T>::EnvelopeSignedByUnknownValidator); | ||
ebma marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Check if all envelopes are using the same slot index | ||
ensure!( | ||
slot_index == envelope.statement.slot_index, | ||
Error::<T>::EnvelopeSlotIndexMismatch | ||
); | ||
|
||
let signature_valid = verify_signature(envelope, &node_id, network); | ||
ensure!(signature_valid, Error::<T>::InvalidEnvelopeSignature); | ||
} | ||
|
||
// Check if transaction set matches tx_set_hash included in the ScpEnvelopes | ||
let expected_tx_set_hash = compute_non_generic_tx_set_content_hash(transaction_set) | ||
.ok_or(Error::<T>::FailedToComputenonGenericTxSetContentHash)?; | ||
.ok_or(Error::<T>::FailedToComputeNonGenericTxSetContentHash)?; | ||
|
||
// We store the externalized value in a variable so that we can check if it's the same | ||
// for all envelopes. We don't distinguish between externalized and confirmed values as | ||
// it should be the same value regardless. | ||
let (externalized_value, externalized_n_h) = | ||
match &externalized_envelope.statement.pledges { | ||
ScpStatementPledges::ScpStExternalize(externalized_statement) => | ||
(&externalized_statement.commit.value, externalized_statement.n_h), | ||
ScpStatementPledges::ScpStConfirm(confirmed_statement) => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This case cannot happen as |
||
(&confirmed_statement.ballot.value, confirmed_statement.n_h), | ||
_ => return Err(Error::<T>::ExternalizedValueNotFound), | ||
}; | ||
|
||
for envelope in envelopes.get_vec() { | ||
match envelope.clone().statement.pledges { | ||
ScpStatementPledges::ScpStExternalize(externalized_statement) => { | ||
let tx_set_hash = Self::get_tx_set_hash(&externalized_statement)?; | ||
ensure!( | ||
tx_set_hash == expected_tx_set_hash, | ||
Error::<T>::TransactionSetHashMismatch | ||
); | ||
}, | ||
let (value, n_h) = match &envelope.statement.pledges { | ||
ScpStatementPledges::ScpStExternalize(externalized_statement) => | ||
(&externalized_statement.commit.value, externalized_statement.n_h), | ||
ScpStatementPledges::ScpStConfirm(confirmed_statement) => | ||
(&confirmed_statement.ballot.value, confirmed_statement.n_h), | ||
_ => return Err(Error::<T>::InvalidScpPledge), | ||
TorstenStueber marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
}; | ||
|
||
// Check if the tx_set_hash matches the one included in the envelope | ||
let tx_set_hash = Self::get_tx_set_hash(&value)?; | ||
ensure!( | ||
tx_set_hash == expected_tx_set_hash, | ||
Error::<T>::TransactionSetHashMismatch | ||
); | ||
|
||
// Check if the externalized value is the same for all envelopes | ||
ensure!(externalized_value == value, Error::<T>::ExternalizedValueMismatch); | ||
ensure!(externalized_n_h == n_h, Error::<T>::ExternalizedNHMismatch); | ||
} | ||
|
||
// ---- Check that externalized messages build valid quorum set ---- | ||
|
@@ -656,9 +701,8 @@ pub mod pallet { | |
Ok(()) | ||
} | ||
|
||
fn get_tx_set_hash(x: &ScpStatementExternalize) -> Result<Hash, Error<T>> { | ||
let scp_value = x.commit.value.get_vec(); | ||
let tx_set_hash = StellarValue::from_xdr(scp_value) | ||
fn get_tx_set_hash(scp_value: &Value) -> Result<Hash, Error<T>> { | ||
let tx_set_hash = StellarValue::from_xdr(scp_value.get_vec()) | ||
.map(|stellar_value| stellar_value.tx_set_hash) | ||
.map_err(|_| Error::<T>::TransactionSetHashCreationFailed)?; | ||
Ok(tx_set_hash) | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe revert back to
sp_std
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think this matters? It changed because the latest template file uses
core::