Skip to content

feat: grant role timelock - changesets and EVM sequence#104

Open
ecPablo wants to merge 3 commits into
mainfrom
ecpablo/grant-role-timelock
Open

feat: grant role timelock - changesets and EVM sequence#104
ecPablo wants to merge 3 commits into
mainfrom
ecpablo/grant-role-timelock

Conversation

@ecPablo

@ecPablo ecPablo commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Adds the refactored version of the grant role changeset, using the mcms lib on the operations and all the helpers and structure established by the new refactored changesets.

@github-actions

Copy link
Copy Markdown

Release impact (release-please)

Current version 0.7.1 (on main)
After merge 0.8.0 (minor bump)

PR title: feat: grant role timelock - changesets and EVM sequence

Merging this PR as-is will contribute a minor bump to the next release-please release PR.

Conventional commit → bump

Intent PR title prefix Bump
Bug fix fix: patch
New feature feat: minor
Breaking change feat!: / fix!: or BREAKING CHANGE: / BREAKING-CHANGE: in description major
No release chore:, docs:, ci:, refactor:, etc. none

Update the PR title before merge if you need a different bump (squash commit message = PR title).

Preview is based on this PR title only. The release-please release PR may include other unreleased commits already on main.

@ecPablo ecPablo changed the base branch from main to ecpablo/fund-mcms-pdas June 26, 2026 01:34
ecPablo added a commit that referenced this pull request Jun 26, 2026
`opsutils.go` is only used by legacy code. The only usages in non-legacy
mcms is currentlly in `grant-roles` pkg, which will be refactored in
this PR #104
Base automatically changed from ecpablo/fund-mcms-pdas to main June 26, 2026 06:17
@ecPablo ecPablo marked this pull request as ready for review June 26, 2026 15:02
@ecPablo ecPablo requested a review from a team as a code owner June 26, 2026 15:02
@ecPablo ecPablo requested review from graham-chainlink and removed request for ChrisAmora June 26, 2026 15:02
@ecPablo ecPablo force-pushed the ecpablo/grant-role-timelock branch from f128632 to 19d82f9 Compare June 26, 2026 15:12
@cl-sonarqube-production

Copy link
Copy Markdown

Comment on lines +41 to +45
families := make([]string, 0, len(byFamily))
for family := range byFamily {
families = append(families, family)
}
slices.Sort(families)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

could use map.Slice with map.Keys

type Config struct {
GrantsByChain map[uint64][]RoleGrant `json:"grantsByChain"`
// GasBoostConfig optionally configures EVM retry gas boosting for direct sends.
GasBoostConfig *proposalutils.GasBoostConfig `json:"gasBoostConfig,omitempty"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do you think we can use the new GasBoostConfig here? I think long term we want to replace all usage of proposalutils.GasBoostConfig with the newer one.

ChainSelector uint64 `json:"chainSelector"`
Grants []RoleGrant `json:"grants"`
MCMS *cldf.MCMSTimelockProposalInput `json:"mcms,omitempty"`
GasBoostConfig *proposalutils.GasBoostConfig `json:"gasBoostConfig,omitempty"`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Here too

if in.NoSend {
opts = cldf.SimTransactOpts()
} else {
opts = gasboost.CloneTransactOptsWithGas(deps.DeployerKey, in.GasLimit, in.GasPrice)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm maybe we should export this function since the logic is being duplicated here too? (not a blocker to this PR).


if in.MCMS == nil {
return nil
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should we move this check up to the first few lines of this method, then we no longer need to do line 41-43 nil check again?

Comment on lines +104 to +106
func parseTimelockAddress(raw string) (common.Address, error) {
return parseEVMAddress(raw)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

any reason to keep this? It doesnt add any new logic?

selector: cldfevm.Chain{Selector: selector},
}),
DataStore: ds.Seal(),
GetContext: context.Background,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
GetContext: context.Background,
GetContext: t.Context,

return cldf.Environment{
Logger: logger.Nop(),
DataStore: ds,
GetContext: context.Background,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
GetContext: context.Background,
GetContext: t.Context,

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.

2 participants