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

Prevent invalid destination griefing for the relayer #2703

Merged
merged 30 commits into from
Sep 19, 2023

Conversation

aroralanuk
Copy link
Contributor

Description

  • Adds destinationDomain to the GasPayment event
  • recording destination domain as destination while reading events for the relayer

Drive-by changes

  • none

Related issues

Backward compatibility

No, change in event emitted and relayer indexing

Testing

  • Unit

yorhodes and others added 20 commits July 25, 2023 16:22
### Description

- Adding Optimism Hook for MailboxV3

### Drive-by changes

- dispatch function with hookMetadata

### Related issues

- hyperlane-xyz/issues#513

### Backward compatibility

No

### Testing

None
### Description

- IGP as a standalone hook, implementing postDispatch to call payForGas
directly
- Setting a DEFAULT_GAS_USAGE if metadata not specified and
message.senderAddress() as refund address if not specified.

### Drive-by changes

- None

### Related issues

Fixes hyperlane-xyz/issues#511

### Backward compatibility

Yes, same interface as the previous IGP but for Mailbox V3

### Testing

Unit Tests

---------

Co-authored-by: Yorke Rhodes <[email protected]>
### Description

- Updated the OP Stack tests for the Mailbox V3 transient storage
version
- Allowing OPStackHook to send msg.value at the time of message delivery
(uses bit masking)
- Added LibBit library, will be useful for all the auth hooks which can
send `msg.value`

### Drive-by changes

- None

### Related issues

- Fixes breaking OP Stack tests for
hyperlane-xyz/issues#513
- Also fixes
#2410

### Backward compatibility

No

### Testing

Unit tests

---------

Co-authored-by: Yorke Rhodes <[email protected]>
### Description

- Adding protocol fee as a hook

### Drive-by changes

- None

### Related issues

V3

### Backward compatibility

Yes

### Testing

Fuzz tests

---------

Co-authored-by: Yorke Rhodes <[email protected]>
### Description

- fixes GasRouter expectRevert message
- added TestMerkleRootHook for proof() and fixes MerkleRootMultisig test
- fixes ERC5164 tests post v3 changes

### Drive-by changes

- added contract name to mailbox and abstractHook error messages
- removed unnecessary tests for "message too large"
- downgrade slither in actions to 0.3.0 because of their recent "missing
inheritance" bug on main

### Related issues

- V3

### Backward compatibility

Yes

### Testing

Unit tests
### Description

- `quoteDispatch` added to `IPostDispatchHook` interface and the
relevant hooks:
     - StaticProtocolFee
     - OPStackHook
     - InterchainGasPaymaster
     - MerkleTreeHook
     - PausableHook (no tests)
     - DomainRoutingHook (no tests)
     - ConfigFallbackDomainRoutingHook 
     - ConfigurableDomainRoutingHook (no tests)

### Drive-by changes

- `expectEmit` -> `expectCall`

### Related issues

- Quote in V3

### Backward compatibility

Yes

### Testing

Uint tests
### Description

- AggregationHook for v3

### Drive-by changes

None

### Related issues

v3

hyperlane-xyz/issues#514

### Backward compatibility

Yes

### Testing

Always be fuzzing
Signed-off-by: -f <[email protected]>
yorhodes and others added 3 commits September 8, 2023 18:22
### Description

- V3 compatible ERC20, ERC721 and all their extensions

### Drive-by changes

- Painstakingly migrating hardhat tests to foundry tests for all
variants

### Related issues

- fixes hyperlane-xyz/issues#577

### Backward compatibility

Yes

### Testing

whole lotta unit tests and poor man's version of integration tests

---------

Signed-off-by: -f <[email protected]>
Co-authored-by: Yorke Rhodes <[email protected]>
@codecov
Copy link

codecov bot commented Sep 14, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (7312a6f) 49.32% compared to head (bea2b4f) 49.32%.

Additional details and impacted files
@@           Coverage Diff           @@
##               v3    #2703   +/-   ##
=======================================
  Coverage   49.32%   49.32%           
=======================================
  Files         107      107           
  Lines        1417     1417           
  Branches      179      179           
=======================================
  Hits          699      699           
  Misses        711      711           
  Partials        7        7           
Files Changed Coverage Δ
solidity/contracts/igps/InterchainGasPaymaster.sol 77.77% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@aroralanuk
Copy link
Contributor Author

Issues right now:

  • do we need destination in the struct InterchainGasPayment. It's missing in sealevel right now
  • retrieve_interchain_gas_payment_data_by_message_id doesn't work as intended, something wrong with encoding for make_store_and_retrieve

@aroralanuk aroralanuk requested a review from tkporter September 15, 2023 17:01
Copy link
Collaborator

@tkporter tkporter left a comment

Choose a reason for hiding this comment

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

LGTM! Nice stuff

@aroralanuk aroralanuk merged commit f0e4f2b into v3 Sep 19, 2023
12 checks passed
@aroralanuk aroralanuk deleted the prevent-invalid-dest-griefing branch September 19, 2023 14:30
@yorhodes yorhodes mentioned this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants