Skip to content
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

Replace message_id with nonce in MessageProof query #1363

Merged
merged 9 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,17 @@ Description of the upcoming release here.
- [#1342](https://github.com/FuelLabs/fuel-core/pull/1342): Add error handling for P2P requests to return `None` to requester and log error

### Breaking
- [#1363](https://github.com/FuelLabs/fuel-core/pull/1363): Change message_proof api to take `nonce` instead of `message_id`
- [#1355](https://github.com/FuelLabs/fuel-core/pull/1355): Removed the `metrics` feature flag from the fuel-core crate, and metrics are now included by default.
- [#1318](https://github.com/FuelLabs/fuel-core/pull/1318): Removed the `--sync-max-header-batch-requests` CLI argument, and renamed `--sync-max-get-txns` to `--sync-block-stream-buffer-size` to better represent the current behavior in the import.
- [#1290](https://github.com/FuelLabs/fuel-core/pull/1290): Standardize CLI args to use `-` instead of `_`.
- [#1279](https://github.com/FuelLabs/fuel-core/pull/1279): Added a new CLI flag to enable the Relayer service `--enable-relayer`, and disabled the Relayer service by default. When supplying the `--enable-relayer` flag, the `--relayer` argument becomes mandatory, and omitting it is an error. Similarly, providing a `--relayer` argument without the `--enable-relayer` flag is an error. Lastly, providing the `--keypair` or `--network` arguments will also produce an error if the `--enable-p2p` flag is not set.
- [#1262](https://github.com/FuelLabs/fuel-core/pull/1262): The `ConsensusParameters` aggregates all configuration data related to the consensus. It contains many fields that are segregated by the usage. The API of some functions was affected to use lesser types instead the whole `ConsensusParameters`. It is a huge breaking change requiring repetitively monotonically updating all places that use the `ConsensusParameters`. But during updating, consider that maybe you can use lesser types. Usage of them may simplify signatures of methods and make them more user-friendly and transparent.
- [#1322](https://github.com/FuelLabs/fuel-core/pull/1322):
The `debug` flag is added to the CLI. The flag should be used for local development only. Enabling debug mode:
- Allows GraphQL Endpoints to arbitrarily advance blocks.
- Enables debugger GraphQL Endpoints.
- Allows setting `utxo_validation` to `false`.
- [#1318](https://github.com/FuelLabs/fuel-core/pull/1318): Removed the `--sync-max-header-batch-requests` CLI argument, and renamed `--sync-max-get-txns` to `--sync-block-stream-buffer-size` to better represent the current behavior in the import.
- [#1290](https://github.com/FuelLabs/fuel-core/pull/1290): Standardize CLI args to use `-` instead of `_`.
- [#1279](https://github.com/FuelLabs/fuel-core/pull/1279): Added a new CLI flag to enable the Relayer service `--enable-relayer`, and disabled the Relayer service by default. When supplying the `--enable-relayer` flag, the `--relayer` argument becomes mandatory, and omitting it is an error. Similarly, providing a `--relayer` argument without the `--enable-relayer` flag is an error. Lastly, providing the `--keypair` or `--network` arguments will also produce an error if the `--enable-p2p` flag is not set.
- [#1262](https://github.com/FuelLabs/fuel-core/pull/1262): The `ConsensusParameters` aggregates all configuration data related to the consensus. It contains many fields that are segregated by the usage. The API of some functions was affected to use lesser types instead the whole `ConsensusParameters`. It is a huge breaking change requiring repetitively monotonically updating all places that use the `ConsensusParameters`. But during updating, consider that maybe you can use lesser types. Usage of them may simplify signatures of methods and make them more user-friendly and transparent.

### Removed

Expand Down
4 changes: 1 addition & 3 deletions crates/client/assets/schema.sdl
Original file line number Diff line number Diff line change
Expand Up @@ -542,8 +542,6 @@ type MessageEdge {
node: Message!
}

scalar MessageId

type MessageProof {
messageProof: MerkleProof!
blockProof: MerkleProof!
Expand Down Expand Up @@ -696,7 +694,7 @@ type Query {
contractBalances(filter: ContractBalanceFilterInput!, first: Int, after: String, last: Int, before: String): ContractBalanceConnection!
nodeInfo: NodeInfo!
messages(owner: Address, first: Int, after: String, last: Int, before: String): MessageConnection!
messageProof(transactionId: TransactionId!, messageId: MessageId!, commitBlockId: BlockId, commitBlockHeight: U32): MessageProof
messageProof(transactionId: TransactionId!, nonce: Nonce!, commitBlockId: BlockId, commitBlockHeight: U32): MessageProof
}

type Receipt {
Expand Down
7 changes: 3 additions & 4 deletions crates/client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ use fuel_core_types::{
fuel_types::{
canonical::SerializedSize,
BlockHeight,
MessageId,
Nonce,
},
};
Expand Down Expand Up @@ -879,18 +878,18 @@ impl FuelClient {
pub async fn message_proof(
&self,
transaction_id: &TxId,
message_id: &MessageId,
nonce: &Nonce,
commit_block_id: Option<&BlockId>,
commit_block_height: Option<BlockHeight>,
) -> io::Result<Option<types::MessageProof>> {
let transaction_id: TransactionId = (*transaction_id).into();
let message_id: schema::MessageId = (*message_id).into();
let nonce: schema::Nonce = (*nonce).into();
let commit_block_id: Option<schema::BlockId> =
commit_block_id.map(|commit_block_id| (*commit_block_id).into());
let commit_block_height = commit_block_height.map(Into::into);
let query = schema::message::MessageProofQuery::build(MessageProofArgs {
transaction_id,
message_id,
nonce,
commit_block_id,
commit_block_height,
});
Expand Down
5 changes: 2 additions & 3 deletions crates/client/src/client/schema/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::client::{
schema::{
schema,
Address,
MessageId,
Nonce,
U32,
U64,
Expand Down Expand Up @@ -81,7 +80,7 @@ pub struct OwnedMessagesConnectionArgs {
pub struct MessageProofQuery {
#[arguments(
transactionId: $transaction_id,
messageId: $message_id,
nonce: $nonce,
commitBlockId: $commit_block_id,
commitBlockHeight: $commit_block_height
)]
Expand Down Expand Up @@ -128,7 +127,7 @@ pub struct MessageProofArgs {
/// Transaction id that contains the output message.
pub transaction_id: TransactionId,
/// Message id of the output message that requires a proof.
pub message_id: MessageId,
pub nonce: Nonce,

/// The query supports either `commit_block_id`, or `commit_block_height` set on, not both.

Expand Down
1 change: 0 additions & 1 deletion crates/client/src/client/schema/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ fuel_type_scalar!(AssetId, AssetId);
fuel_type_scalar!(ContractId, ContractId);
fuel_type_scalar!(Salt, Salt);
fuel_type_scalar!(TransactionId, Bytes32);
fuel_type_scalar!(MessageId, MessageId);
fuel_type_scalar!(Signature, Bytes64);
fuel_type_scalar!(Nonce, Nonce);

Expand Down
7 changes: 5 additions & 2 deletions crates/fuel-core/src/query/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use fuel_core_types::{
},
fuel_merkle::binary::in_memory::MerkleTree,
fuel_tx::{
input::message::compute_message_id,
Receipt,
TxId,
},
Expand Down Expand Up @@ -139,7 +140,7 @@ impl<D: DatabasePort + ?Sized> MessageProofData for D {
pub fn message_proof<T: MessageProofData + ?Sized>(
database: &T,
transaction_id: Bytes32,
message_id: MessageId,
desired_nonce: Nonce,
commit_block_id: BlockId,
) -> StorageResult<Option<MessageProof>> {
// Check if the receipts for this transaction actually contain this message id or exit.
Expand All @@ -154,7 +155,7 @@ pub fn message_proof<T: MessageProofData + ?Sized>(
amount,
data,
..
} if r.message_id() == Some(message_id) => {
} if r.nonce() == Some(&desired_nonce) => {
Some((sender, recipient, nonce, amount, data))
}
_ => None,
Expand Down Expand Up @@ -185,6 +186,8 @@ pub fn message_proof<T: MessageProofData + ?Sized>(
None => return Ok(None),
};

let message_id = compute_message_id(&sender, &recipient, &nonce, amount, &data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this is kinda the meat of the change: Rather than using the message id directly, we pass in the nonce. Together, these pieces of information allow us to reconstruct the message id.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. Maybe I could have reworded the contents of the issue.

We were just asking for more information than needed.


let message_proof =
match message_receipts_proof(database, message_id, &message_block_txs)? {
Some(proof) => proof,
Expand Down
17 changes: 10 additions & 7 deletions crates/fuel-core/src/query/message/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ async fn can_build_message_proof() {
let commit_block_height = BlockHeight::from(2u32);
let message_block_height = BlockHeight::from(1u32);
let expected_receipt = receipt(Some(11));
let message_id = expected_receipt.message_id().unwrap();
let nonce = expected_receipt.nonce().unwrap();
let receipts: [Receipt; 4] = [
receipt(Some(10)),
receipt(None),
receipt(Some(3)),
expected_receipt,
expected_receipt.clone(),
];
static TXNS: [Bytes32; 4] = [txn_id(20), txn_id(24), txn_id(1), txn_id(33)];
let transaction_id = TXNS[3];
Expand Down Expand Up @@ -199,11 +199,14 @@ async fn can_build_message_proof() {

let data: Box<dyn MessageProofData> = Box::new(data);

let proof =
message_proof(data.deref(), transaction_id, message_id, commit_block.id())
.unwrap()
.unwrap();
assert_eq!(proof.message_id(), message_id);
let proof = message_proof(
data.deref(),
transaction_id,
nonce.to_owned(),
commit_block.id(),
)
.unwrap()
.unwrap();
assert_eq!(
proof.message_block_header.message_receipt_root,
message_block.header().message_receipt_root
Expand Down
5 changes: 2 additions & 3 deletions crates/fuel-core/src/schema/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use super::{
Address,
Bytes32,
HexString,
MessageId,
Nonce,
TransactionId,
U64,
Expand Down Expand Up @@ -118,7 +117,7 @@ impl MessageQuery {
&self,
ctx: &Context<'_>,
transaction_id: TransactionId,
message_id: MessageId,
nonce: Nonce,
commit_block_id: Option<BlockId>,
commit_block_height: Option<U32>,
) -> async_graphql::Result<Option<MessageProof>> {
Expand All @@ -137,7 +136,7 @@ impl MessageQuery {
Ok(crate::query::message_proof(
data.deref(),
transaction_id.into(),
message_id.into(),
nonce.into(),
block_id,
)?
.map(MessageProof))
Expand Down
12 changes: 6 additions & 6 deletions tests/tests/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,16 @@ async fn can_get_message_proof() {
let message_ids: Vec<_> =
receipts.iter().filter_map(|r| r.message_id()).collect();

// Get the nonces from the receipt
let nonces: Vec<_> = receipts.iter().filter_map(|r| r.nonce()).collect();

// Check we actually go the correct amount of ids back.
assert_eq!(message_ids.len(), args.len(), "{receipts:?}");
assert_eq!(nonces.len(), args.len(), "{receipts:?}");

for message_id in message_ids.clone() {
for nonce in nonces.clone() {
// Request the proof.
let result = client
.message_proof(&transaction_id, &message_id, None, Some(last_height))
.message_proof(&transaction_id, nonce, None, Some(last_height))
.await
.unwrap()
.unwrap();
Expand All @@ -402,9 +405,6 @@ async fn can_get_message_proof() {
&result.data,
);

// Check message id is the same as the one passed in.
assert_eq!(generated_message_id, message_id);

// 2. Generate the block id. (full header)
let mut hasher = Hasher::default();
hasher.input(result.message_block_header.prev_root.as_ref());
Expand Down