feat: implement TimelockProposal.Merge() #446
Conversation
🦋 Changeset detectedLatest commit: 16c817b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
gustavogama-cll
force-pushed
the
ggama/feat-timelockproposal-merge
branch
from
92da715 to
bdd173c
Compare
| } | ||
|
|
||
| return ChainMetadata{ | ||
| StartingOpCount: min(m.StartingOpCount, other.StartingOpCount), |
There was a problem hiding this comment.
it's not clear to me if we should reject merging a proposal if m.StartingOpCount != (other.StartingOpCount + m.TxCount).
There was a problem hiding this comment.
I'd say we should be able to merge them, as long as the opcount on chain does not change. We can reset the opcount to the on chain value and add the txsCounts in both proposals ignoring whatever was there in the first place?
There was a problem hiding this comment.
ok. Let's go with the min() logic, for now. But I don't think we should handle opcounts in the Merge method -- we have dedicated logic to handle that elsewhere.
gustavogama-cll
requested review from
ChrisAmora
and removed request for
skudasov
| } | ||
|
|
||
| return ChainMetadata{ | ||
| StartingOpCount: min(m.StartingOpCount, other.StartingOpCount), |
There was a problem hiding this comment.
I'd say we should be able to merge them, as long as the opcount on chain does not change. We can reset the opcount to the on chain value and add the txsCounts in both proposals ignoring whatever was there in the first place?
gustavogama-cll
force-pushed
the
ggama/feat-timelockproposal-merge
branch
from
fdbc061 to
16c817b
Compare
|
|
addressed all comments from reviews |
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @smartcontractkit/mcms@0.22.0 ### Minor Changes - [#441](#441) [`fdf66b3`](fdf66b3) Thanks [@ecPablo](https://github.com/ecPablo)! - Adds support for getMinDelay to timelock inspectors on all supported chain families - [#446](#446) [`2e56b50`](2e56b50) Thanks [@gustavogama-cll](https://github.com/gustavogama-cll)! - feat: implement TimelockProposal.Merge() - [#450](#450) [`6e542cb`](6e542cb) Thanks [@ecPablo](https://github.com/ecPablo)! - feat: add converted operations count function ### Patch Changes - [#449](#449) [`8f14da3`](8f14da3) Thanks [@ecPablo](https://github.com/ecPablo)! - fix: avoid requiring all executors for converters generation Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
2 participants
DX-1499