Conversation
🦋 Changeset detectedLatest commit: d7e5d1b 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 |
product-security-plaid-production
bot
requested a review
from DimitriosNaikopoulos
sdk/aptos/inspector.go
Outdated
| func NewInspector(client aptos.AptosRpcClient, role TimelockRole) *Inspector { | ||
| // NewInspector creates an Inspector. When isCurseMCMS is true the Inspector | ||
| // talks to a CurseMCMS contract; when false it talks to a standard MCMS contract. | ||
| func NewInspector(client aptos.AptosRpcClient, role TimelockRole, isCurseMCMS bool) *Inspector { |
There was a problem hiding this comment.
i believe we can try not to break this interface, by introducing functional option pattern, and pass isCurseMCMS as an option. Then we dont have to update many places with false as the 3rd param
ecPablo
left a comment
ecPablo
left a comment
There was a problem hiding this comment.
I'm approving with the caveat of the product specific logic just to unblock
But I'd check this with @friedemannf or other folks from the aptos team
sdk/aptos/detect.go
Outdated
| // given chain selector has package_name == "curse_mcms" in its additional | ||
| // fields. This is a reliable self-describing signal already present in every | ||
| // CurseMCMS proposal. | ||
| func IsCurseMCMSFromOperations(ops []types.BatchOperation, cs types.ChainSelector) bool { |
There was a problem hiding this comment.
I'm not sure if its the best idea to get ccip specific logic in this package? Is there a way to provide this in a more generic way via a string o similar or let the client logic handle this?
|
| } | ||
|
|
||
| func NewExecutor(client aptos.AptosRpcClient, auth aptos.TransactionSigner, encoder *Encoder, role TimelockRole) *Executor { | ||
| func NewExecutor(client aptos.AptosRpcClient, auth aptos.TransactionSigner, encoder *Encoder, role TimelockRole, mcmsType MCMSType) *Executor { |
There was a problem hiding this comment.
this needs to be an optional argument, else you'll break the public interface
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.36.4 ### Patch Changes - [#650](#650) [`c037579`](c037579) Thanks [@ecPablo](https://github.com/ecPablo)! - fix garbage characters getting printed in the derivation path during ledger signing - [#663](#663) [`279dae9`](279dae9) Thanks [@FelixFan1992](https://github.com/FelixFan1992)! - make executor interface backwards compatible - [#658](#658) [`f3650e8`](f3650e8) Thanks [@FelixFan1992](https://github.com/FelixFan1992)! - enhance the aptos mcms inspector with different mcms (regular and curse) Co-authored-by: app-token-issuer-engops[bot] <144731339+app-token-issuer-engops[bot]@users.noreply.github.com>
4 participants
No description provided.