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

Add Access Control, Refunder and Rate Limiter #772

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Primata
Copy link
Contributor

@Primata Primata commented Oct 30, 2024

Summary

  • RFCs: Link to RFC, Link to RFC, or $\emptyset$.
  • Categories: any of protocol-units, networks, scripts, util, cicd, or misc.

Adds AccessControlUpgradeable to BridgeInitiator and BridgeCounterparty

Changelog

DEFAULT_ADMIN_ROLE: _owner (is able to set roles and is able to set addresses and config)
RELAYER: _relayer (is able to call lockBridgeTransfer)
REFUNDER: _refunder (is able to call cancelBridgeTransfer and refundBridgeTransfer)

RateLiiter

Testing

  1. cd bridge/contracts
  2. forge test

Outstanding issues

Pending tests related to the overall deployment within the relayer framework testing.

@Primata Primata changed the title sketch Add Access Control and Separate Roles Nov 11, 2024
Copy link
Contributor

@0xmovses 0xmovses left a comment

Choose a reason for hiding this comment

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

We need the new abis moved into abis/ directory also.

@Primata
Copy link
Contributor Author

Primata commented Nov 12, 2024

@0xmovses added abis

@0xmovses
Copy link
Contributor

Thanks, it seems that fixed client integration tests are imminently about to be merged to main, EOD / tomorrow morn. I would think its best for those fixed integration tests to go in then you get test these contract changes against main and we merge after that.

Unless there is some other urgency I'm unaware of?

@0xmovses
Copy link
Contributor

This is the PR fyi, https://github.com/movementlabsxyz/movement/pull/690/commits

Some rather large changes required with the way we test against the framework. Old testing work had to be rewritten.

@0xmovses 0xmovses marked this pull request as draft November 20, 2024 14:17
@0xmovses
Copy link
Contributor

Putting this into draft as it refers to the old HTLC contracts, however we may want to refer to this code, so keeping it draft.

@Primata Primata changed the title Add Access Control and Separate Roles Add Access Control, Refunder and Rate Limiter Nov 22, 2024
@Primata Primata changed the base branch from 0xmovses/bridge-contract-deploy to main November 22, 2024 17:25
@Primata Primata marked this pull request as ready for review November 22, 2024 17:25
@andygolay andygolay self-requested a review November 22, 2024 19:21
Copy link
Contributor

@andygolay andygolay left a comment

Choose a reason for hiding this comment

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

LGTM, tests pass locally and logic seems to make sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace OwnableUpgradeable with AccessControlUpgradeable in Bridge Contracts
4 participants