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

refactor: removing msg-key #4856

Merged
merged 7 commits into from
Mar 1, 2024
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
4 changes: 2 additions & 2 deletions docs/docs/developers/debugging/sandbox-errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ Users may create a proof against a historical state in Aztec. The rollup circuit

## Archiver Errors

- "L1 to L2 Message with key ${messageKey.toString()} not found in the confirmed messages store" - happens when the L1 to L2 message doesn't exist or is "pending", when the user has sent a message on L1 via the Inbox contract but it has yet to be included in an L2 block by the sequencer - user has to wait for sequencer to pick it up and the archiver to sync the respective L2 block. You can get the sequencer to pick it up by doing an arbitrary transaction on L2 (eg send DAI to yourself). This would give the sequencer a transaction to process and as a side effect it would look for any pending messages it should include.
- "L1 to L2 Message with key ${entryKey.toString()} not found in the confirmed messages store" - happens when the L1 to L2 message doesn't exist or is "pending", when the user has sent a message on L1 via the Inbox contract but it has yet to be included in an L2 block by the sequencer - user has to wait for sequencer to pick it up and the archiver to sync the respective L2 block. You can get the sequencer to pick it up by doing an arbitrary transaction on L2 (eg send DAI to yourself). This would give the sequencer a transaction to process and as a side effect it would look for any pending messages it should include.

- "Unable to remove message: L1 to L2 Message with key ${messageKeyBigInt} not found in store" - happens when trying to confirm a non-existent pending message or cancelling such a message. Perhaps the sequencer has already confirmed the message?
- "Unable to remove message: L1 to L2 Message with key ${entryKeyBigInt} not found in store" - happens when trying to confirm a non-existent pending message or cancelling such a message. Perhaps the sequencer has already confirmed the message?

- "Block number mismatch: expected ${l2BlockNum} but got ${block.number}" - The archiver keeps track of the next expected L2 block number. It throws this error if it got a different one when trying to sync with the rollup contract's events on L1.

Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ library Constants {
0x85864497636cf755ae7bde03f267ce01a520981c21c3682aaf82a631;
uint256 internal constant DEPLOYER_CONTRACT_ADDRESS =
0x0747a20ed0c86035e44ea5606f30de459f40b55c5e82012640aa554546af9044;
uint256 internal constant L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 25;
uint256 internal constant L1_TO_L2_MESSAGE_ORACLE_CALL_LENGTH = 17;
uint256 internal constant MAX_NOTE_FIELDS_LENGTH = 20;
uint256 internal constant GET_NOTE_ORACLE_RETURN_LENGTH = 23;
uint256 internal constant MAX_NOTES_PER_PAGE = 10;
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/portals/GasPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ contract GasPortal {
fee: _fee
});
bytes32 entryKey = inbox.cancelL2Message(message, address(this));
// release the funds to msg.sender (since the content hash (& message key) is derived by hashing the caller,
// release the funds to msg.sender (since the content hash) is derived by hashing the caller,
// we confirm that msg.sender is same as `_canceller` supplied when creating the message)
underlying.transfer(msg.sender, _amount);
return entryKey;
Expand Down
4 changes: 2 additions & 2 deletions l1-contracts/test/portals/TokenPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ contract TokenPortal {
fee: _fee
});
bytes32 entryKey = inbox.cancelL2Message(message, address(this));
// release the funds to msg.sender (since the content hash (& message key) is derived by hashing the caller,
// release the funds to msg.sender (since the content hash (& entry key) is derived by hashing the caller,
// we confirm that msg.sender is same as `_canceller` supplied when creating the message)
underlying.transfer(msg.sender, _amount);
return entryKey;
Expand Down Expand Up @@ -175,7 +175,7 @@ contract TokenPortal {
fee: _fee
});
bytes32 entryKey = inbox.cancelL2Message(message, address(this));
// release the funds to msg.sender (since the content hash (& message key) is derived by hashing the caller,
// release the funds to msg.sender (since the content hash (& entry key) is derived by hashing the caller,
// we confirm that msg.sender is same as `_canceller` supplied when creating the message)
underlying.transfer(msg.sender, _amount);
return entryKey;
Expand Down
64 changes: 32 additions & 32 deletions l1-contracts/test/portals/UniswapPortal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,10 @@ contract UniswapPortalTest is Test {
entryKey = outbox.computeEntryKey(message);
}

function _addMessagesToOutbox(bytes32 daiWithdrawMessageKey, bytes32 swapMessageKey) internal {
function _addMessagesToOutbox(bytes32 daiWithdrawEntryKey, bytes32 swapEntryKey) internal {
bytes32[] memory entryKeys = new bytes32[](2);
entryKeys[0] = daiWithdrawMessageKey;
entryKeys[1] = swapMessageKey;
entryKeys[0] = daiWithdrawEntryKey;
entryKeys[1] = swapEntryKey;
vm.prank(address(rollup));

outbox.sendL1Messages(entryKeys);
Expand Down Expand Up @@ -215,10 +215,10 @@ contract UniswapPortalTest is Test {
}

function testRevertIfSwapParamsDifferentToOutboxMessage() public {
bytes32 daiWithdrawMsgKey =
bytes32 daiWithdrawEntryKey =
_createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal));
bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey);
bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);

bytes32 newAztecRecipient = bytes32(uint256(0x4));
bytes32 entryKeyPortalChecksAgainst =
Expand All @@ -241,12 +241,12 @@ contract UniswapPortalTest is Test {
}

function testSwapWithDesignatedCaller() public {
bytes32 daiWithdrawMsgKey =
bytes32 daiWithdrawEntryKey =
_createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal));
bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey);
bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);

bytes32 l1ToL2MessageKey = uniswapPortal.swapPublic(
bytes32 l1ToL2EntryKey = uniswapPortal.swapPublic(
address(daiTokenPortal),
amount,
uniswapFeePool,
Expand All @@ -264,22 +264,22 @@ contract UniswapPortalTest is Test {
// there should be some weth in the weth portal
assertGt(WETH9.balanceOf(address(wethTokenPortal)), 0);
// there should be a message in the inbox:
assertEq(inbox.get(l1ToL2MessageKey).count, 1);
assertEq(inbox.get(l1ToL2EntryKey).count, 1);
// there should be no message in the outbox:
assertFalse(outbox.contains(daiWithdrawMsgKey));
assertFalse(outbox.contains(swapMsgKey));
assertFalse(outbox.contains(daiWithdrawEntryKey));
assertFalse(outbox.contains(swapEntryKey));
}

function testSwapCalledByAnyoneIfDesignatedCallerNotSet(address _caller) public {
vm.assume(_caller != address(uniswapPortal));
bytes32 daiWithdrawMsgKey =
bytes32 daiWithdrawEntryKey =
_createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal));
// don't set caller on swapPublic() -> so anyone can call this method.
bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(0));
_addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey);
bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(0));
_addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);

vm.prank(_caller);
bytes32 l1ToL2MessageKey = uniswapPortal.swapPublic(
bytes32 l1ToL2EntryKey = uniswapPortal.swapPublic(
address(daiTokenPortal),
amount,
uniswapFeePool,
Expand All @@ -297,18 +297,18 @@ contract UniswapPortalTest is Test {
// there should be some weth in the weth portal
assertGt(WETH9.balanceOf(address(wethTokenPortal)), 0);
// there should be a message in the inbox:
assertEq(inbox.get(l1ToL2MessageKey).count, 1);
assertEq(inbox.get(l1ToL2EntryKey).count, 1);
// there should be no message in the outbox:
assertFalse(outbox.contains(daiWithdrawMsgKey));
assertFalse(outbox.contains(swapMsgKey));
assertFalse(outbox.contains(daiWithdrawEntryKey));
assertFalse(outbox.contains(swapEntryKey));
}

function testRevertIfSwapWithDesignatedCallerCalledByWrongCaller(address _caller) public {
vm.assume(_caller != address(this));
bytes32 daiWithdrawMsgKey =
bytes32 daiWithdrawEntryKey =
_createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal));
bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey);
bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);

vm.startPrank(_caller);
bytes32 entryKeyPortalChecksAgainst = _createUniswapSwapMessagePublic(aztecRecipient, _caller);
Expand Down Expand Up @@ -352,12 +352,12 @@ contract UniswapPortalTest is Test {
// if the sequencer doesn't consume the L1->L2 message, then `canceller` can
// cancel the message and retrieve the funds (instead of them being stuck on the portal)
function testMessageToInboxIsCancellable() public {
bytes32 daiWithdrawMsgKey =
bytes32 daiWithdrawEntryKey =
_createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal));
bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey);
bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);

bytes32 l1ToL2MessageKey = uniswapPortal.swapPublic{value: 1 ether}(
bytes32 l1ToL2EntryKey = uniswapPortal.swapPublic{value: 1 ether}(
address(daiTokenPortal),
amount,
uniswapFeePool,
Expand All @@ -376,13 +376,13 @@ contract UniswapPortalTest is Test {
// check event was emitted
vm.expectEmit(true, false, false, false);
// expected event:
emit L1ToL2MessageCancelled(l1ToL2MessageKey);
emit L1ToL2MessageCancelled(l1ToL2EntryKey);
// perform op
// TODO(2167) - Update UniswapPortal properly with new portal standard.
bytes32 entryKey = wethTokenPortal.cancelL1ToAztecMessagePublic(
aztecRecipient, wethAmountOut, deadlineForL1ToL2Message, secretHash, 1 ether
);
assertEq(entryKey, l1ToL2MessageKey, "returned entry key and calculated entryKey should match");
assertEq(entryKey, l1ToL2EntryKey, "returned entry key and calculated entryKey should match");
assertFalse(inbox.contains(entryKey), "entry still in inbox");
assertEq(
WETH9.balanceOf(address(this)),
Expand All @@ -393,12 +393,12 @@ contract UniswapPortalTest is Test {
}

function testRevertIfSwapMessageWasForDifferentPublicOrPrivateFlow() public {
bytes32 daiWithdrawMsgKey =
bytes32 daiWithdrawEntryKey =
_createDaiWithdrawMessage(address(uniswapPortal), address(uniswapPortal));

// Create message for `_isPrivateFlow`:
bytes32 swapMsgKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawMsgKey, swapMsgKey);
bytes32 swapEntryKey = _createUniswapSwapMessagePublic(aztecRecipient, address(this));
_addMessagesToOutbox(daiWithdrawEntryKey, swapEntryKey);

bytes32 entryKeyPortalChecksAgainst =
_createUniswapSwapMessagePrivate(secretHashForRedeemingMintedNotes, address(this));
Expand Down
3 changes: 1 addition & 2 deletions noir-projects/aztec-nr/aztec/src/context/private_context.nr
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,14 @@ impl PrivateContext {

// docs:start:context_consume_l1_to_l2_message
// docs:start:consume_l1_to_l2_message
pub fn consume_l1_to_l2_message(&mut self, msg_key: Field, content: Field, secret: Field, sender: EthAddress) {
pub fn consume_l1_to_l2_message(&mut self, content: Field, secret: Field, sender: EthAddress) {
// docs:end:context_consume_l1_to_l2_message
let nullifier = process_l1_to_l2_message(
self.historical_header.state.l1_to_l2_message_tree.root,
self.this_address(),
sender,
self.chain_id(),
self.version(),
msg_key,
content,
secret
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,14 @@ impl PublicContext {
self.new_l2_to_l1_msgs.push(message);
}

pub fn consume_l1_to_l2_message(&mut self, msg_key: Field, content: Field, secret: Field, sender: EthAddress) {
pub fn consume_l1_to_l2_message(&mut self, content: Field, secret: Field, sender: EthAddress) {
let this = (*self).this_address();
let nullifier = process_l1_to_l2_message(
self.historical_header.state.l1_to_l2_message_tree.root,
this,
sender,
self.chain_id(),
self.version(),
msg_key,
content,
secret
);
Expand Down
56 changes: 18 additions & 38 deletions noir-projects/aztec-nr/aztec/src/messaging.nr
Original file line number Diff line number Diff line change
@@ -1,58 +1,38 @@
mod l1_to_l2_message;
mod l1_to_l2_message_getter_data;

use l1_to_l2_message_getter_data::make_l1_to_l2_message_getter_data;

use crate::oracle::get_l1_to_l2_message::get_l1_to_l2_message_call;
use crate::oracle::get_l1_to_l2_membership_witness::get_l1_to_l2_membership_witness;

use dep::std::merkle::compute_merkle_root;
use crate::messaging::l1_to_l2_message::L1ToL2Message;
use dep::protocol_types::{constants::L1_TO_L2_MSG_TREE_HEIGHT, address::{AztecAddress, EthAddress}, utils::arr_copy_slice};

use dep::protocol_types::address::{AztecAddress, EthAddress};

// Returns the nullifier for the message
pub fn process_l1_to_l2_message(
l1_to_l2_root: Field,
storage_contract_address: AztecAddress,
portal_contract_address: EthAddress,
chain_id: Field,
version: Field,
msg_key: Field,
content: Field,
secret: Field
) -> Field {
let returned_message = get_l1_to_l2_message_call(msg_key);
let l1_to_l2_message_data = make_l1_to_l2_message_getter_data(returned_message, 0, secret);
let msg = L1ToL2Message::new(
portal_contract_address,
chain_id,
storage_contract_address,
version,
content,
secret
);
let entry_key = msg.hash();

// Check that the returned message is actually the message we looked up
let msg_hash = l1_to_l2_message_data.message.hash();
assert(msg_hash == msg_key, "Message not matching requested key");
let returned_message = get_l1_to_l2_membership_witness(entry_key);
let leaf_index = returned_message[0];
let sibling_path = arr_copy_slice(returned_message, [0; L1_TO_L2_MSG_TREE_HEIGHT], 1);

// Check that the message is in the tree
let root = compute_merkle_root(
msg_hash,
l1_to_l2_message_data.leaf_index,
l1_to_l2_message_data.sibling_path
);
// This is implicitly checking that the values of the message are correct
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool!

let root = compute_merkle_root(entry_key, leaf_index, sibling_path);
assert(root == l1_to_l2_root, "Message not in state");

// Validate this is the target contract
assert(l1_to_l2_message_data.message.recipient.eq(storage_contract_address), "Invalid recipient");

// Validate the sender is the portal contract
assert(l1_to_l2_message_data.message.sender.eq(portal_contract_address), "Invalid sender");

// Validate the chain id is correct
assert(l1_to_l2_message_data.message.chainId == chain_id, "Invalid Chainid");

// Validate the version is correct
assert(l1_to_l2_message_data.message.version == version, "Invalid Version");

// Validate the message hash is correct
assert(l1_to_l2_message_data.message.content == content, "Invalid Content");

// Validate the message secret is correct
l1_to_l2_message_data.message.validate_message_secret();

// Compute Nullifier
l1_to_l2_message_data.message.compute_nullifier()
msg.compute_nullifier()
}
38 changes: 29 additions & 9 deletions noir-projects/aztec-nr/aztec/src/messaging/l1_to_l2_message.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ use dep::protocol_types::{
hash::{pedersen_hash, sha256_to_field}
};

// TODO(#4833) remove `deadline` and `fee` from the message
// currently hardcoded to max_value and 0 respectively.
struct L1ToL2Message {
sender: EthAddress,
chainId: Field,
chain_id: Field,
recipient: AztecAddress,
version: Field,
content: Field,
Expand All @@ -18,14 +20,37 @@ struct L1ToL2Message {
}

impl L1ToL2Message {
pub fn new(
sender: EthAddress,
chain_id: Field,
recipient: AztecAddress,
version: Field,
content: Field,
secret: Field
) -> L1ToL2Message {
let secret_hash = pedersen_hash([secret], GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET);
Self {
sender,
chain_id,
recipient,
version,
content,
secret,
secret_hash,
deadline: 4294967295,
fee: 0,
tree_index: 0
}
}

pub fn deserialize(
fields: [Field; L1_TO_L2_MESSAGE_LENGTH],
secret: Field,
tree_index: Field
) -> L1ToL2Message {
L1ToL2Message {
sender: EthAddress::from_field(fields[0]),
chainId: fields[1],
chain_id: fields[1],
recipient: AztecAddress::from_field(fields[2]),
version: fields[3],
content: fields[4],
Expand All @@ -37,15 +62,10 @@ impl L1ToL2Message {
}
}

pub fn validate_message_secret(self: Self) {
let recomputed_hash = pedersen_hash([self.secret], GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET);
assert(self.secret_hash == recomputed_hash, "Invalid message secret");
}

fn hash(self: Self) -> Field {
let mut hash_bytes: [u8; 256] = [0; 256];
let sender_bytes = self.sender.to_field().to_be_bytes(32);
let chainId_bytes = self.chainId.to_be_bytes(32);
let chain_id_bytes = self.chain_id.to_be_bytes(32);
let recipient_bytes = self.recipient.to_field().to_be_bytes(32);
let version_bytes = self.version.to_be_bytes(32);
let content_bytes = self.content.to_be_bytes(32);
Expand All @@ -55,7 +75,7 @@ impl L1ToL2Message {

for i in 0..32 {
hash_bytes[i] = sender_bytes[i];
hash_bytes[i + 32] = chainId_bytes[i];
hash_bytes[i + 32] = chain_id_bytes[i];
hash_bytes[i + 64] = recipient_bytes[i];
hash_bytes[i + 96] = version_bytes[i];
hash_bytes[i + 128] = content_bytes[i];
Expand Down
Loading
Loading