feat: add get_messages_status

[jbcaron]

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

• Feature

What is the current behavior?

Resolves: #NA

What is the new behavior?

Does this introduce a breaking change?

No

Other information

[cchudant]

Missing failure_reason / execution_status fields.

[cchudant]

i think we're supposed to return something meaningful when the transaction is not on l2 here

[cchudant]

Alright - I am looking at the latest v0.9 spec. I also looked at the v0.8 rpc spec but I figured I would focus on v0.9 spec because we'll have to do that as well, and the corner cases are similar.

https://github.com/starkware-libs/starknet-specs/blob/release/v0.9.0-rc.3/api/starknet_api_openrpc.json

Here is the interpretation of the spec.

• If the transaction can't be found on L1, TxnHashNotFound (correct)

• finality_status: this can be PreConfirmed, AcceptedOnL1 and AcceptedOnL2
  You are fine here since it's get_transaction_status that handles that (esp. AcceptedOnL1).
  I don't think we're supposed to return Received status here (because messages from l1 txs can't be in the mempool) nor Candidate status because execution_status is required.

• execution_status: you are missing this field. This field is returned by get_transaction_status, so you should just pass it to the resulting DTO. Weirdly enough, this field is required, so we can't return finality_status Candidate in this endpoint. I should bring this up with the spec authors tbh, it's probably something they haven't thought about.

• failure_reason: you are also missing this field. This is the revert reason, and you can also get it from the output of get_transaction_status where it's also named failure_reason. It's optional and should be None for the other statuses.

• What if the transaction is on L1 and not on l2/preconfirmed? This is not as far-fetched of an idea as you may think, the sequencer can totally just omit L1HandlerTransactions and/or miss them.
  Either, 1) none of the messages are on l2 yet. 2) the rare case where one of the messages to l2 is on l2 but one of the others are missing.
  I don't know what the proper interpretation is here - if finality_status was nullable I would say we should return a None finality_status. Since it's not, I think skipping the transaction makes sense. If we were allowed to return a null execution_status, I think it could also kinda make sense to return Received.
  I've looked at how pathfinder interprets the spec, and it's a bit weird? Here is how they interpret it: if a get_transaction_status call for one of the messages returns TxnHashNotFound, the whole getMessagesStatus returns TxnHashNotFound. Which means, the endpoint can't be used at all when case 2) hits, like, if a message is skipped by the sequencer somehow or if one of the messages arrives on l2 before another (could happen for a lot of reason). But here is the weird part: if a get_transaction_status call returns Candidate (or Received), the given transaction is skipped from the output because there is no associated execution_status. This feels weird to me? i am super confused.

---

I think I need to bring this up to the spec authors, but in the meantime, my opinion is that you should:

• add the execution_status and finality_status fields to match the spec. Just use what get_transaction_status returns here.
• I think it kind of make sense to return TxnHashNotFound like you did if a transaction is not on l2 in the meantime, to match pathfinder's interpretation. I think it would be weird for users to get an empty array if their tx has not hit l2 yet, but I haven't checked how consumers use the API.
• candidate shenanigans might require spec change, let's worry about that later when we're doing v0.9.

[jbcaron]

this implementation is only for v0.8.1

[cchudant]

oh btw,

stream::iter(messages).then(|msg| async move {
  let transaction_hash = msg.tx.compute_hash(starknet.chain_id(), false, false);
  let finality_status = get_transaction_status(starknet, transaction_hash).await?.finality_status;
  Ok(TxnHashWithStatus { transaction_hash, finality_status })
}).collect().await

should work

(https://docs.rs/futures/latest/futures/stream/index.html, and https://docs.rs/futures/latest/futures/stream/trait.StreamExt.html#method.then which is the async version of iterator map)

(it doesn't change anything, the fetches will still be sequential but i think it looks nicer? maybe overcomplicated for nothing idk)

[jbcaron]

there’s a slight overhead with the stream I think, and I’m not sure there’s more than 1 message per TX