Skip to content

Public Contract interfaces #190

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 21, 2021
Merged

Public Contract interfaces #190

merged 2 commits into from
Jan 21, 2021

Conversation

aalavandhan
Copy link
Member

@aalavandhan aalavandhan commented Jan 19, 2021 •
edited
Loading

  • Checked in standard public interface definition for, AMPL token, Ampleforth monetary policy and Orchestrator.

@aalavandhan aalavandhan force-pushed the adding-interfaces branch 3 times, most recently from 2b71be8 to df1bea3 Compare January 19, 2021 17:39
@aalavandhan aalavandhan changed the title Checked-in standard public interface Checked-in public interface Jan 19, 2021
brandoniles
brandoniles reviewed Jan 19, 2021
View reviewed changes
Copy link
Member

@brandoniles brandoniles left a comment

Choose a reason for hiding this comment

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

Should we include the Orchestrator? That has the main rebase callsite

contracts/interfaces/IAMPL.sol Outdated
function transferAllFrom(address from, address to) external returns (bool);

// AMPL specific
function rebase(uint256 epoch, int256 supplyDelta) external returns (uint256);
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering whether we should include this since it's protected, but I think we should.
The Supply Policy would ideally import the interface to call this function from code.
The other protected functions are admin functions that would be called generically from the governance contract.
👍

Copy link
Member Author

@aalavandhan aalavandhan Jan 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense 👍
We can have a seperate interface for governance accessible functions

@aalavandhan aalavandhan changed the title Checked-in public interface Cross-chain AMPL standard getters Jan 20, 2021
@aalavandhan aalavandhan requested a review from thegostep January 20, 2021 16:27
@aalavandhan aalavandhan force-pushed the adding-interfaces branch 3 times, most recently from d922ce9 to 0f5e9a3 Compare January 20, 2021 18:40
@aalavandhan
Copy link
Member Author

@brandoniles Added the orchestrator too

@aalavandhan aalavandhan changed the title Cross-chain AMPL standard getters Cross-chain AMPL standard getters & Contract interfaces Jan 20, 2021
brandoniles
brandoniles reviewed Jan 20, 2021
View reviewed changes
@aalavandhan aalavandhan changed the title Cross-chain AMPL standard getters & Contract interfaces Public Contract interfaces Jan 20, 2021
brandoniles
brandoniles approved these changes Jan 20, 2021
View reviewed changes
Copy link
Member

@brandoniles brandoniles left a comment

Choose a reason for hiding this comment

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

Looks good. Curious to hear @thegostep 's take as well.

@aalavandhan aalavandhan merged commit dd0465a into master Jan 21, 2021
@aalavandhan aalavandhan deleted the adding-interfaces branch January 21, 2021 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandoniles brandoniles brandoniles approved these changes

@ahnaguib ahnaguib Awaiting requested review from ahnaguib

@thegostep thegostep Awaiting requested review from thegostep

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aalavandhan @brandoniles