-
Notifications
You must be signed in to change notification settings - Fork 43
[PoC] Message Queue contract #789
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
base: main
Are you sure you want to change the base?
Conversation
| g.executionState.AppendForwardTransaction(txn) | ||
| } | ||
|
|
||
| messages, err := GetMessageQueueContent(g.executionState) |
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.
Shouldn't we get messages queue from other shards targeting to our shard?
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.
We should do that by proposer. I suggest to implement step-by-step solution. It's the first step in this directon.
Right now an approach doesn't break existing code. We just copy a message queue content to an existing outbound messages tree. Ideally, we need to drop old approach and then it will be possible to use only queue account state.
| delete queue; | ||
| } | ||
|
|
||
| Message[] private queue; |
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 it is worth to separate this array by shard. So the other shard can fetch only messages addressed to him: getMessages(uint shardId)
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.
Right now we don't have a special interface to send data to specific shard. Our smart accounts operates with raw calldata and we can extract destination shard only after decoding a message. But we don't have a SSZ decoder in Solidity. So here we have single queue for all messages. In future then we will pass destination shards/address explicitly it will be possible.
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.
What are the limits on the size of the queue or message size in queue?
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.
Also how would this compare to another approach such as emitting an event instead?
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.
What are the limits on the size of the queue or message size in queue?
Right now there is no any limits but we can add them
Also how would this compare to another approach such as emitting an event instead?
I didn't think about emitting events but this looks like an attempt to move all magic out of EVM. Sounds interesting but I'm not sure it's what we expect. Should be discussed. Thanks for idea.
| bytes data; | ||
| address sender; |
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.
it may be convenient to introduce more fields here (e.g. tokens)
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.
In the first version I'd just want to check that it will work at all. basic test passed. We can discussed it and extends current implementation.
| if err != nil { | ||
| return fmt.Errorf("failed to create MQ prune transaction: %w", err) | ||
| } | ||
| p.proposal.SpecialTxns = append(p.proposal.SpecialTxns, txn) |
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.
What happens if the tx fails or if the block containing the tx is not finalized?
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.
If such TX fail we fail block validation on consensus and then new block in the next round will be suggested.
| delete queue; | ||
| } | ||
|
|
||
| Message[] private queue; |
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.
What are the limits on the size of the queue or message size in queue?
| delete queue; | ||
| } | ||
|
|
||
| Message[] private queue; |
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.
Also how would this compare to another approach such as emitting an event instead?
| } | ||
|
|
||
| contract MessageQueue is IMessageQueue{ | ||
| function sendRawTransaction(bytes calldata _message) external override { |
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.
What are thoughts around supporting attaching value?
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.
Value is attached inside this message. It's out InternalTransactionPayload encoded with SSZ.
We can try to send messages encoded in EVM serialization but it's also should be discussed.
Do we need to send SSZ inside EVM at all. Or should be accept only ABI packed data and only then we need to repack into SSZ
There are several issues that arise with this approach, and not all of them have been resolved yet.
Summary of Changes
SmartAccount.sendmethod is ported to this approach.Potential Next Steps
Nil.sendTransactioninterface withMessageQueue.sendRawTransaction.