feat(rpc): v0.8.0 getMessagesStatus method
#388
Conversation
shamsasari
force-pushed
the
shams-get_msgs_status
branch
from
9aaeb24 to
b69a042
Compare
shamsasari
changed the title
getMessagesStatus method
|
|
shamsasari
force-pushed
the
shams-get_msgs_status
branch
from
b69a042 to
b02aacc
Compare
shamsasari
force-pushed
the
shams-get_msgs_status
branch
from
b02aacc to
7346894
Compare
# Conflicts: # CHANGELOG.md # crates/client/db/src/lib.rs
| &self, | ||
| l1_tx_hash: TxHash, | ||
| l1_handler_tx_hash: Felt, | ||
| order: u64, |
There was a problem hiding this comment.
Can the index be a type smaller than u64?
There was a problem hiding this comment.
The reason it's u64 is because it's using the log index. Not sure why that needs to be that big, even u32 is more than enough to cover logs in a block I would have thought. I can reduce if you think it's safe.
| return Ok(()); | ||
| } | ||
|
|
||
| let l1_handler_transaction = parse_handle_l1_message_transaction(&event)?; |
There was a problem hiding this comment.
I wonder if it would be better to use our own primitive, mp_transaction::L1HandlerTransaction, even if it requires integrating starknet-api for blocking purposes.
| } | ||
| let mut message_statuses = vec![]; | ||
| for l1_handler_tx_hash in l1_handler_tx_hashes { | ||
| let finality_status = match get_transaction_status(starknet, l1_handler_tx_hash) { |
There was a problem hiding this comment.
I think we will need to review the get_transaction_status function in sequencer mode, as we need to handle transactions in both the mempool and the pending state
I think that it maybe a good idea to wait for the rolling updates PR from @Trantorian1 to merge this one. |
# Conflicts: # CHANGELOG.md # crates/client/eth/src/l1_messaging.rs # crates/client/rpc/src/versions/user/v0_8_0/methods/read/get_messages_status.rs # crates/client/rpc/src/versions/user/v0_8_0/methods/read/lib.rs
It picks up from the last log event that was processed (which is stored in the db). So yes, I think it does. |
| key[..32].copy_from_slice(l1_tx_hash.as_slice()); | ||
| key[32..].copy_from_slice(&order.to_be_bytes()); // BE is important for the lexographic sorting | ||
| let mut writeopts = WriteOptions::default(); | ||
| writeopts.disable_wal(true); |
There was a problem hiding this comment.
Shouldn't WAL be enabled so we don't risk loosing this on a node restart?
There was a problem hiding this comment.
Storing the mapping is guarded behind messaging_update_last_synced_l1_block_with_event, which also has WAL disabled. The sync will restart from the last stored LastSyncedEventBlock. So I think that should also be enabled? @cchudant
crates/client/rpc/src/versions/user/v0_8_0/methods/read/get_messages_status.rs
Outdated
Show resolved
Hide resolved
|
PTAL |
Pull Request type
What is the current behavior?
Resolves: #414
What is the new behavior?
starket_getMessagesStatusRPC method--analytics_collection_endpointconfig was also fixedDoes this introduce a breaking change?
New database column added.
Other information
Since there can be multiple handler transactions per L1 transaction, and because the processing of L1 -> L2 messaging is done on a per log event basis, a composite DB key is used so that new entries for the same L1 tx can be added without having to re-serialize and update the same key. Retrieving the handler tx hashes is done by a prefix scan on the L1 transaction hash which is an efficient operation with RocksDB.
Since rejections are currently not supported, the RPC response doesn't populate the
failure_reasonfield.