Skip to content

Primata/contract pipeline - #1285

Merged
0xmovses merged 10 commits into
mainfrom
primata/contract-pipeline
Aug 28, 2025
Merged

Primata/contract pipeline#1285
0xmovses merged 10 commits into
mainfrom
primata/contract-pipeline

Conversation

@Primata

@Primata Primata commented Jul 29, 2025

Copy link
Copy Markdown
Contributor

Summary

  • RFCs: $\emptyset$.
  • Categories: protocol-units.

We had to deploy a new Timelock with new proposers and executors.

Changelog

Updates addresses on config.json to reflect new addresses.
After fixing foundry.toml we are able to bring back solidity settlement/contracts so it compiles correctly.

Testing

cd protocol-units/settlement/mcr/contracts
forge test

Outstanding issues

MCR is incomplete according to test. But it's not blocking.
Further verify deployments to make sure that no capabilities or configurations wrongly affected the protocol.

Copilot AI review requested due to automatic review settings July 29, 2025 04:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR appears to be updating the Movement staking contract pipeline with various optimizations and configuration changes. The changes focus on code cleanup, deployment configuration updates, and build optimization.

Key changes include:

  • Removed unused debug imports and simplified variable usage in smart contracts
  • Updated deployment addresses for Movement Foundation Safe and timelock contracts
  • Enhanced build configuration with optimizer settings

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
MovementStakingStorage.sol Removed unused forge-std console import
MCR.sol Eliminated unnecessary intermediate variable in attester loop
deployments.json Updated Foundation Safe and timelock contract addresses
config.json Fixed minDelay type from string to number
Helper.sol Refactored timelock deployment to use actual addresses instead of config arrays
foundry.toml Added optimizer configuration for gas efficiency

Comment thread protocol-units/settlement/mcr/contracts/script/helpers/Helper.sol
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@0xmovses

Copy link
Copy Markdown
Contributor

Could you add a description describing these changes and why we need them (paper trail) Also the foundry test cmd (it's been a while since we've run these locally). There are two conflicts here against main.

@Primata

Primata commented Jul 29, 2025

Copy link
Copy Markdown
Contributor Author

Could you add a description describing these changes and why we need them (paper trail) Also the foundry test cmd (it's been a while since we've run these locally). There are two conflicts here against main.

Yeah, waiting on a couple of things to finalize this PR

@Primata

Primata commented Jul 29, 2025

Copy link
Copy Markdown
Contributor Author

@0xmovses brought fixes for you

@Primata Primata mentioned this pull request Aug 21, 2025
@0xmovses

Copy link
Copy Markdown
Contributor

As you mentioned in the PR description, some tests are failing

[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testChangingValidatorSet() (gas: 9223372036854754743)
[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testDishonestValidator() (gas: 683128)
[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testRollsOverHandlingDishonesty() (gas: 791043)
[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testSimpleStaking() (gas: 683105)

If we expect these to fail, can you add expect failure for them? Also could you explain why each test should fail at this point? Thanks!

Comment thread protocol-units/settlement/mcr/contracts/script/helpers/config.json
@Primata

Primata commented Aug 21, 2025

Copy link
Copy Markdown
Contributor Author

As you mentioned in the PR description, some tests are failing

[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testChangingValidatorSet() (gas: 9223372036854754743)
[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testDishonestValidator() (gas: 683128)
[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testRollsOverHandlingDishonesty() (gas: 791043)
[FAIL. Reason: revert: UNAUTHORIZED_BLOCK_COMMITMENT] testSimpleStaking() (gas: 683105)

If we expect these to fail, can you add expect failure for them? Also could you explain why each test should fail at this point? Thanks!

commented out failing tests that are not applicable.

@0xmovses

Copy link
Copy Markdown
Contributor

Will be swinging back to review this.

@0xmovses 0xmovses left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@0xmovses 0xmovses left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@0xmovses

Copy link
Copy Markdown
Contributor

@apenzk pinging for re-review

@apenzk

apenzk commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

The PR provides an incomplete PR description. In particular it ommits that it already introduces the dev version of the MOVETokenV2 contract

I suggest to improve the PR description :


## Summary
We had to deploy a new Timelock with new proposers and executors.
It also prepares the deployment and upgrade flow for moving the existing MOVE token to a mint-enabled implementation in a future PR.

## Changes
- Add `CoreDeployer` to orchestrate conditional deploy/upgrade of MOVE, staking, stlMOVE, and MCR.
- Add deployers for individual components (`MOVETokenDeployer`, `MultisigMOVETokenDeployer`, `MovementStakingDeployer`, `StlMoveDeployer`, `MCRDeployer`).
- Add placeholder `MOVETokenDev` contract with mint roles (`MINTER_ROLE`, `MINTER_ADMIN_ROLE`) and role management helpers for upcoming V2.
- Update helper scripts, config, and deployments JSON.
- Tests:
  - Base MOVE token tests (`MOVEToken.t.sol`) covering proxy deploy, timelock upgrade, and admin role checks.
  - **`MOVETokenV2.t.sol` (MOVETokenDevTest) included to validate minting and role logic ahead of the actual upgrade.**

Comment thread protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol Outdated
Comment thread protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol Outdated
Comment thread protocol-units/settlement/mcr/contracts/test/token/MOVETokenV2.t.sol Outdated
@apenzk
apenzk self-requested a review August 28, 2025 05:46

@apenzk apenzk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

forge test works for me.

Would prefer if there would be more documentation on the tests and i think the PR description could be improved, see #1285 (comment)

but this is non-blocking.

@0xmovses
0xmovses merged commit d224a0a into main Aug 28, 2025
36 of 44 checks passed
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.

5 participants