Skip to content

improve(cross-chain-oracle): Clarify comments for refundL2Address#3687

Merged
chrismaree merged 3 commits intomasterfrom
npai/cross-chain-comments
Dec 14, 2021
Merged

improve(cross-chain-oracle): Clarify comments for refundL2Address#3687
chrismaree merged 3 commits intomasterfrom
npai/cross-chain-comments

Conversation

@nicholaspai
Copy link
Copy Markdown
Member

Motivation

Comments are misleading about what the refundL2Address should be set to. Clarifies that address should be set to an EOA or contract that can spend the funds.

Testing

Check a box to describe how you tested these changes and list the steps for reviewers to test.

  • Ran end-to-end test, running the code as in production
  • New unit tests created
  • Existing tests adequate, no new tests required
  • All existing tests pass
  • Untested

@nicholaspai nicholaspai added audit-fix-phase6 Fixes for Phase 6 (Cross-chain oracle, optimistic rewarder, decentralized proposer) cross-chain-oracle Contracts enabling cross-chain price resolution and governance labels Dec 11, 2021
Copy link
Copy Markdown
Member

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

LGTM! One minor comment

* @dev The caller of this function must be the owner. This should be set to the DVM governor.
* @param newRefundl2Address the new refund address to set.
* @dev The caller of this function must be the owner, which should be set to the DVM governor.
* @param newRefundl2Address the new refund address to set. This should be set to an L2 address that is trusted by
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When we say trusted, what do we mean? This address just gets the excess funds? Or does it have more control than that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that's right. it can get the excess funds on L2.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I updated this comment to be a bit more clear.

Signed-off-by: chrismaree <christopher.maree@gmail.com>
@chrismaree chrismaree merged commit 7e0c5e0 into master Dec 14, 2021
@chrismaree chrismaree deleted the npai/cross-chain-comments branch December 14, 2021 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

audit-fix-phase6 Fixes for Phase 6 (Cross-chain oracle, optimistic rewarder, decentralized proposer) cross-chain-oracle Contracts enabling cross-chain price resolution and governance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants