Skip to content

feat: implement getMessageReceipt and tests#1955

Merged
smartcontracts merged 1 commit intodevelopfrom
sc/sdk-msg-receipt
Dec 21, 2021
Merged

feat: implement getMessageReceipt and tests#1955
smartcontracts merged 1 commit intodevelopfrom
sc/sdk-msg-receipt

Conversation

@smartcontracts
Copy link
Contributor

Description
Implements the getMessageReceipt function and corresponding tests. Also adds a new toCrossChainMessage function that can parse a MessageLike into a CrossChainMessage object.

Metadata

  • Fixes ENG-1800

@changeset-bot
Copy link

changeset-bot bot commented Dec 21, 2021

⚠️ No Changeset found

Latest commit: c8a68b7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@tynes
Copy link
Contributor

tynes commented Dec 21, 2021

With all of the todos, is this a draft or ready for review?

@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2021

Codecov Report

Merging #1955 (03caf3e) into develop (1c4f838) will increase coverage by 2.47%.
The diff coverage is n/a.

❗ Current head 03caf3e differs from pull request most recent head 42d3e65. Consider uploading reports for the commit 42d3e65 to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1955      +/-   ##
===========================================
+ Coverage    73.13%   75.61%   +2.47%     
===========================================
  Files           79       57      -22     
  Lines         2520     1993     -527     
  Branches       387      294      -93     
===========================================
- Hits          1843     1507     -336     
+ Misses         677      486     -191     
Flag Coverage Δ
batch-submitter 62.50% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer ?
message-relayer ?
sdk ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/sdk/src/cross-chain-provider.ts
packages/sdk/src/interfaces/types.ts
packages/sdk/src/utils/index.ts
...ckages/data-transport-layer/src/utils/constants.ts
...kages/data-transport-layer/src/utils/validation.ts
...layer/src/services/l1-ingestion/handlers/errors.ts
...ices/l1-ingestion/handlers/transaction-enqueued.ts
packages/sdk/src/utils/coercion.ts
packages/data-transport-layer/src/utils/index.ts
packages/sdk/src/interfaces/index.ts
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c4f838...42d3e65. Read the comment docs.

@smartcontracts
Copy link
Contributor Author

With all of the todos, is this a draft or ready for review?

The TODOs are mainly just notes for myself in the future, nothing blocking this PR. Going to run over all of the TODOs later and make tickets out of them if they're actually worth doing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be === ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to find the first SentMessage event such that the logIndex is greater than the TokenBridgeMessage logIndex. Basically we detect TokenBridgeMessage events separately (they're usually stuff like ERC20DepositInitiated or WithdrawalInitiated). These events are then followed by a SentMessage event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can add a comment here to make this more clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a detailed comment that explains what's happening here

@smartcontracts smartcontracts force-pushed the sc/sdk-msg-receipt branch 2 times, most recently from 03caf3e to 42d3e65 Compare December 21, 2021 19:49
Copy link

@SmitV SmitV left a comment

Choose a reason for hiding this comment

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

Approving based on review on call. Good stuff

Copy link

Choose a reason for hiding this comment

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

Not part of the PR but I before E except after C :)

@smartcontracts smartcontracts merged commit dddc160 into develop Dec 21, 2021
@smartcontracts smartcontracts deleted the sc/sdk-msg-receipt branch December 21, 2021 23:08
theochap pushed a commit that referenced this pull request Dec 10, 2025
### Description

Bumps dependency versions to latest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants