Skip to content

Conversation

roxdanila
Copy link
Contributor

Context

Closes issue #620

Changes proposed in this pull request

  • make all functions that would return misleading info return
  • remove unused dependencies
  • switch to MasterAware2 - TBD if feasible

Test plan

Ran existing tests suite

Checklist

  • Rebased the base branch
  • Attached corresponding Github issue
  • Prefixed the name with the type of change (i.e. feat, chore, test)
  • Performed a self-review of my own code
  • Followed the style guidelines of this project
  • Made corresponding changes to the documentation
  • Didn't generate new warnings
  • Didn't generate failures on existing tests
  • Added tests that prove my fix is effective or that my feature works

Review

When reviewing a PR, please indicate intention in comments using the following emojis:

  • 🍰 = Nice to have but not essential.
  • 💡 = Suggestion or a comment based on personal opinion
  • 🔨 = I believe this should be changed.
  • 🤔 = I don’t understand something, do you mind giving me more context?
  • 🚀 = Feedback

@roxdanila roxdanila requested a review from danoctavian January 11, 2023 11:21
@roxdanila roxdanila linked an issue Jan 11, 2023 that may be closed by this pull request
@roxdanila roxdanila changed the base branch from master to feature/staking-nft-contracts January 11, 2023 11:21
@roxdanila roxdanila marked this pull request as ready for review January 11, 2023 12:51
@roxdanila roxdanila requested a review from shark0der January 11, 2023 12:53
Base automatically changed from feature/staking-nft-contracts to nexus-v2 January 11, 2023 13:21
@shark0der shark0der force-pushed the chore/gatway-cleanup branch from 363c011 to 93d4685 Compare January 11, 2023 13:21
// which occupies 2 slots instead of 1.
// address public _unused_quotation;

address public _unused_nxmToken;
Copy link
Contributor

Choose a reason for hiding this comment

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

we've usually been naming them with numbers

but i don't see any issue with this, it's actually self-documenting, seems better

} else {
status = ClaimStatus.IN_PROGRESS;
}
function initializeDAI() external {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need this function? DAI is initialized

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

address public _unused_memberRoles;

// assigned in initialize
address public DAI;
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: We will need to verify storage slots on the fork

@MilGard91 once this is merged.

Based on visual inspection, looks correct.

@roxdanila roxdanila merged commit d6d686a into nexus-v2 Jan 11, 2023
@roxdanila roxdanila deleted the chore/gatway-cleanup branch January 11, 2023 15:56
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.

Cleanup LegacyGateway contract
3 participants