Let USDC look for the exact nonce instead of relying on ordering #1025

RensR · 2024-06-14T13:04:45Z

Motivation

We don't want to have the offchain code be reliant on onchain ordering of events.

Solution

Since we only relied on the ordering of USDC events, we can solve the dependency by removing the ordering requirement for USDC. With this new method we look for the exact nonce, which is significantly less brittle overal. It should not care about log ordering, so we can remove the comment from the onRamps

github-actions · 2024-06-14T13:12:19Z

LCOV of commit `da1ba26` during Solidity Foundry #5441

Summary coverage rate:
  lines......: 98.9% (1478 of 1495 lines)
  functions..: 95.8% (297 of 310 functions)
  branches...: 93.1% (639 of 686 branches)

Files changed coverage rate: n/a

connorwstein · 2024-06-17T00:08:59Z

core/services/ocr2/plugins/ccip/internal/ccipdata/usdc_reader.go

-	// fetch all the usdc logs for the provided tx hash
-	logs, err := u.lp.IndexedLogsByTxHash(
+func (u *USDCReaderImpl) GetUSDCMessageWithNonce(ctx context.Context, nonce [32]byte) ([]byte, error) {
+	logs, err := u.lp.IndexedLogsTopicRange(


hmm not sure I follow this - we want this event https://github.com/circlefin/evm-cctp-contracts/blob/master/src/MessageTransmitter.sol#L41 (MessageSent not MessageReceived)?

This PR doesn't change the event we're looking for, it's still u.usdcMessageSent which is eventSig := utils.Keccak256Fixed([]byte("MessageSent(bytes)"))

Ahhh I see what you're saying. I was looking at

event DepositForBurn( uint64 indexed nonce, address indexed burnToken, uint256 amount, address indexed depositor, bytes32 mintRecipient, uint32 destinationDomain, bytes32 destinationTokenMessenger, bytes32 destinationCaller );

Which is the USDC msg but not the CCTP msg, which is

event MessageSent(bytes message); uint32 _msgVersion, uint32 _msgSourceDomain, uint32 _msgDestinationDomain, uint64 _msgNonce, bytes32 _msgSender, bytes32 _msgRecipient, bytes32 _msgDestinationCaller, bytes memory _msgRawBody emit MessageSent(abi.encodePacked( _msgVersion, _msgSourceDomain, _msgDestinationDomain, _msgNonce, _msgSender, _msgRecipient, _msgDestinationCaller, _msgRawBody ); );

I feel like we can still construct the query for this log though, as the first 5 elements should all be known, so we could construct the query for the first slot using those. For EVM (the only supported networks for USDC anyway) you don't even have to know the msg sender, as it will always be a 160 bit EVM address which adds only 0x000.... to the first slot

_msgVersion is always 0
_msgSourceDomain is known
_msgDestinationDomain is known
_msgNonce is known
_msgSender is known (the pool)

yeah seems possible, not the most straightforward thing though. I think you'd have to account for the offset/length slots of the bytes encoding as well so it'd be 3 words and the length would include the burn message body length, which looks like its static as well (luckily). Also would need to read the dest domain and pool address from the chain to smoothly support adding new dest domains/pool upgrades. I think worth a little exploratory doc/discussion though

We would only have to query for the third slot, which contains the items I mentioned above.

MessageSent [topic0] 0x8c5261668696ce22758910d05bab8f186d6eb247ceac2af2e82c7dc17669b036 0000000000000000000000000000000000000000000000000000000000000020 00000000000000000000000000000000000000000000000000000000000000f8 0000000000000000000000020000000000011c30000000000000000000000000 .....

This is a real MessageSent event emitted on mainnet. As you can see, we'd be interested in the last slot I copied here. Afaik the logPoller simply treats each slot as a queryable object.

0000000000000000000000020000000000011c30000000000000000000000000 => 00000000 00000000 00000002 0000000000011c30 000000000000000000000000 version srcDom dstDom nonce start of sender (evm, so always 000...)

0xnogo · 2024-07-30T08:23:18Z

core/services/ocr2/plugins/ccip/internal/ccipdata/usdc_reader.go

 		ctx,
 		u.usdcMessageSent,
 		u.transmitterAddress,
-		common.HexToHash(txHash),
+		USDC_MESSAGE_NONCE_INDEX,


Why are we getting the topicIndex 3? Shouldn't we have only 1 topic considering that the event doesn't emit indexed value?

event MessageSent(bytes message);

Expect if topicIndex in logpoller actually means slots. Then, I understand that we need to get the 3rd slot (0: event signature, 1: offset to data, 2: length of data, 3: packed data).

0xnogo · 2024-07-30T09:47:29Z

core/services/ocr2/plugins/ccip/internal/ccipdata/usdc_reader.go

+	nonceBytes := [8]byte{}
+	binary.BigEndian.PutUint64(nonceBytes[:], nonce)
+
+	senderBytes := [12]byte{}


The sender address is the pool address. We probably need to have it as an input param (like sourceDomain and destDomain)

We can get the sourceDomain from the MessageTransmitter https://github.com/circlefin/evm-cctp-contracts/blob/master/src/MessageTransmitter.sol#L72. This means we probably need to add method in usdc_reader.

0xnogo · 2024-07-30T10:29:58Z

core/services/ocr2/plugins/ccip/internal/ccipdata/usdc_reader.go

+	nonceBytes := [8]byte{}
+	binary.BigEndian.PutUint64(nonceBytes[:], nonce)
+
+	senderBytes := [12]byte{}


We can get the sourceDomain from the MessageTransmitter https://github.com/circlefin/evm-cctp-contracts/blob/master/src/MessageTransmitter.sol#L72. This means we probably need to add method in usdc_reader.

0xnogo · 2024-07-30T11:02:35Z

core/services/ocr2/plugins/ccip/internal/ccipdata/usdc_reader.go

@@ -119,6 +141,8 @@ func NewUSDCReader(lggr logger.Logger, jobID string, transmitter common.Address,
 			Retention: CommitExecLogsRetention,
 		},
 		transmitterAddress: transmitter,
+		sourceDomain:       0, // TODO
+		destDomain:         1, // TODO


For the destDomain, it cannot be fetched from the source MessageTransmitter contract. I wonder if we can add the MessageTransmitter destination contract address in the config too and call the the domain

github-actions · 2024-09-14T00:15:56Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-11-01T00:19:38Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

Let USDC look for the exact nonce instead of relying on ordering

c92a54c

RensR temporarily deployed to sdlc June 14, 2024 13:04 — with GitHub Actions Inactive

Fix rate limiting unit test.

daaa7d8

winder temporarily deployed to sdlc June 14, 2024 17:42 — with GitHub Actions Inactive

connorwstein reviewed Jun 17, 2024

View reviewed changes

use correct event

da1ba26

RensR temporarily deployed to sdlc June 18, 2024 08:20 — with GitHub Actions Inactive

0xnogo reviewed Jul 30, 2024

View reviewed changes

github-actions bot added the Stale label Sep 14, 2024

RensR removed the Stale label Sep 16, 2024

github-actions bot added the Stale label Nov 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let USDC look for the exact nonce instead of relying on ordering #1025

Let USDC look for the exact nonce instead of relying on ordering #1025

RensR commented Jun 14, 2024 •

edited

Loading

github-actions bot commented Jun 14, 2024 •

edited

Loading

connorwstein Jun 17, 2024

RensR Jun 17, 2024

RensR Jun 17, 2024 •

edited

Loading

RensR Jun 17, 2024 •

edited

Loading

connorwstein Jun 17, 2024

RensR Jun 19, 2024 •

edited

Loading

0xnogo Jul 30, 2024

0xnogo Jul 30, 2024

0xnogo Jul 30, 2024

0xnogo Jul 30, 2024

0xnogo Jul 30, 2024

0xnogo Jul 30, 2024

github-actions bot commented Sep 14, 2024

github-actions bot commented Nov 1, 2024

Let USDC look for the exact nonce instead of relying on ordering #1025

Are you sure you want to change the base?

Let USDC look for the exact nonce instead of relying on ordering #1025

Conversation

RensR commented Jun 14, 2024 • edited Loading

Motivation

Solution

github-actions bot commented Jun 14, 2024 • edited Loading

LCOV of commit da1ba26 during Solidity Foundry #5441

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RensR Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

RensR Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RensR Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 14, 2024

github-actions bot commented Nov 1, 2024

RensR commented Jun 14, 2024 •

edited

Loading

github-actions bot commented Jun 14, 2024 •

edited

Loading

LCOV of commit `da1ba26` during Solidity Foundry #5441

RensR Jun 17, 2024 •

edited

Loading

RensR Jun 17, 2024 •

edited

Loading

RensR Jun 19, 2024 •

edited

Loading