Skip to content

Commit

Permalink
Propagate message verification errors (paritytech#2114)
Browse files Browse the repository at this point in the history
* Propagate message verification errors

* Replace parse_finalized_storage_proof() with storage_proof_checker()

* small fixes

* fix comment
  • Loading branch information
serban300 committed Apr 8, 2024
1 parent 23f568f commit 3642dc4
Show file tree
Hide file tree
Showing 10 changed files with 277 additions and 304 deletions.
218 changes: 92 additions & 126 deletions bridges/bin/runtime-common/src/messages.rs

Large diffs are not rendered by default.

10 changes: 3 additions & 7 deletions bridges/modules/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,11 +1212,8 @@ mod tests {
fn parse_finalized_storage_proof_rejects_proof_on_unknown_header() {
run_test(|| {
assert_noop!(
Pallet::<TestRuntime>::parse_finalized_storage_proof(
Default::default(),
vec![],
|_| (),
),
Pallet::<TestRuntime>::storage_proof_checker(Default::default(), vec![],)
.map(|_| ()),
bp_header_chain::HeaderChainError::UnknownHeader,
);
});
Expand All @@ -1235,8 +1232,7 @@ mod tests {
<ImportedHeaders<TestRuntime>>::insert(hash, header.build());

assert_ok!(
Pallet::<TestRuntime>::parse_finalized_storage_proof(hash, storage_proof, |_| (),),
(),
Pallet::<TestRuntime>::storage_proof_checker(hash, storage_proof).map(|_| ())
);
});
}
Expand Down
20 changes: 12 additions & 8 deletions bridges/modules/messages/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ use bp_messages::{
},
total_unrewarded_messages, DeliveredMessages, InboundLaneData, InboundMessageDetails, LaneId,
MessageKey, MessageNonce, MessagePayload, MessagesOperatingMode, OutboundLaneData,
OutboundMessageDetails, UnrewardedRelayersState,
OutboundMessageDetails, UnrewardedRelayersState, VerificationError,
};
use bp_runtime::{BasicOperatingMode, ChainId, OwnedBridgeModule, PreComputedSize, Size};
use codec::{Decode, Encode, MaxEncodedLen};
Expand Down Expand Up @@ -557,9 +557,9 @@ pub mod pallet {
/// The message is too large to be sent over the bridge.
MessageIsTooLarge,
/// Message has been treated as invalid by chain verifier.
MessageRejectedByChainVerifier,
MessageRejectedByChainVerifier(VerificationError),
/// Message has been treated as invalid by lane verifier.
MessageRejectedByLaneVerifier,
MessageRejectedByLaneVerifier(VerificationError),
/// Submitter has failed to pay fee for delivering and dispatching messages.
FailedToWithdrawMessageFee,
/// The transaction brings too many messages.
Expand Down Expand Up @@ -732,7 +732,7 @@ fn send_message<T: Config<I>, I: 'static>(
err,
);

Error::<T, I>::MessageRejectedByChainVerifier
Error::<T, I>::MessageRejectedByChainVerifier(err)
})?;

// now let's enforce any additional lane rules
Expand All @@ -746,7 +746,7 @@ fn send_message<T: Config<I>, I: 'static>(
err,
);

Error::<T, I>::MessageRejectedByLaneVerifier
Error::<T, I>::MessageRejectedByLaneVerifier(err)
},
)?;

Expand Down Expand Up @@ -918,7 +918,7 @@ impl<T: Config<I>, I: 'static> OutboundLaneStorage for RuntimeOutboundLaneStorag
fn verify_and_decode_messages_proof<Chain: SourceHeaderChain, DispatchPayload: Decode>(
proof: Chain::MessagesProof,
messages_count: u32,
) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, Chain::Error> {
) -> Result<ProvedMessages<DispatchMessage<DispatchPayload>>, VerificationError> {
// `receive_messages_proof` weight formula and `MaxUnconfirmedMessagesAtInboundLane` check
// guarantees that the `message_count` is sane and Vec<Message> may be allocated.
// (tx with too many messages will either be rejected from the pool, or will fail earlier)
Expand Down Expand Up @@ -1177,7 +1177,9 @@ mod tests {
TEST_LANE_ID,
PAYLOAD_REJECTED_BY_TARGET_CHAIN,
),
Error::<TestRuntime, ()>::MessageRejectedByChainVerifier,
Error::<TestRuntime, ()>::MessageRejectedByChainVerifier(VerificationError::Other(
mock::TEST_ERROR
)),
);
});
}
Expand All @@ -1190,7 +1192,9 @@ mod tests {
message.reject_by_lane_verifier = true;
assert_noop!(
send_message::<TestRuntime, ()>(RuntimeOrigin::signed(1), TEST_LANE_ID, message,),
Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier,
Error::<TestRuntime, ()>::MessageRejectedByLaneVerifier(VerificationError::Other(
mock::TEST_ERROR
)),
);
});
}
Expand Down
27 changes: 12 additions & 15 deletions bridges/modules/messages/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use bp_messages::{
ProvedLaneMessages, ProvedMessages, SourceHeaderChain,
},
DeliveredMessages, InboundLaneData, LaneId, Message, MessageKey, MessageNonce, MessagePayload,
OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState,
OutboundLaneData, UnrewardedRelayer, UnrewardedRelayersState, VerificationError,
};
use bp_runtime::{messages::MessageDispatchResult, Size};
use codec::{Decode, Encode};
Expand Down Expand Up @@ -295,22 +295,20 @@ impl Size for TestMessagesDeliveryProof {
pub struct TestTargetHeaderChain;

impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
type Error = &'static str;

type MessagesDeliveryProof = TestMessagesDeliveryProof;

fn verify_message(payload: &TestPayload) -> Result<(), Self::Error> {
fn verify_message(payload: &TestPayload) -> Result<(), VerificationError> {
if *payload == PAYLOAD_REJECTED_BY_TARGET_CHAIN {
Err(TEST_ERROR)
Err(VerificationError::Other(TEST_ERROR))
} else {
Ok(())
}
}

fn verify_messages_delivery_proof(
proof: Self::MessagesDeliveryProof,
) -> Result<(LaneId, InboundLaneData<TestRelayer>), Self::Error> {
proof.0.map_err(|_| TEST_ERROR)
) -> Result<(LaneId, InboundLaneData<TestRelayer>), VerificationError> {
proof.0.map_err(|_| VerificationError::Other(TEST_ERROR))
}
}

Expand All @@ -319,18 +317,16 @@ impl TargetHeaderChain<TestPayload, TestRelayer> for TestTargetHeaderChain {
pub struct TestLaneMessageVerifier;

impl LaneMessageVerifier<RuntimeOrigin, TestPayload> for TestLaneMessageVerifier {
type Error = &'static str;

fn verify_message(
_submitter: &RuntimeOrigin,
_lane: &LaneId,
_lane_outbound_data: &OutboundLaneData,
payload: &TestPayload,
) -> Result<(), Self::Error> {
) -> Result<(), VerificationError> {
if !payload.reject_by_lane_verifier {
Ok(())
} else {
Err(TEST_ERROR)
Err(VerificationError::Other(TEST_ERROR))
}
}
}
Expand Down Expand Up @@ -400,15 +396,16 @@ impl DeliveryConfirmationPayments<AccountId> for TestDeliveryConfirmationPayment
pub struct TestSourceHeaderChain;

impl SourceHeaderChain for TestSourceHeaderChain {
type Error = &'static str;

type MessagesProof = TestMessagesProof;

fn verify_messages_proof(
proof: Self::MessagesProof,
_messages_count: u32,
) -> Result<ProvedMessages<Message>, Self::Error> {
proof.result.map(|proof| proof.into_iter().collect()).map_err(|_| TEST_ERROR)
) -> Result<ProvedMessages<Message>, VerificationError> {
proof
.result
.map(|proof| proof.into_iter().collect())
.map_err(|_| VerificationError::Other(TEST_ERROR))
}
}

Expand Down
Loading

0 comments on commit 3642dc4

Please sign in to comment.