chore: refactor aptos curse converter constructor as a func. option#673
Conversation
🦋 Changeset detectedLatest commit: fc0d2f7 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/chore/refactor-aptos-curse-converters-constructor-as-func-option
branch
from
0162e04 to
e86d0c0
Compare
There was a problem hiding this comment.
Pull request overview
Refactors the Aptos timelock converter construction to use a functional-options pattern (allowing selection of MCMS contract variant) and updates call sites accordingly.
Changes:
- Replaced the dedicated
NewCurseTimelockConverterconstructor withNewTimelockConverter(...TimelockConverterOption)plusWithMCMSType(...). - Updated chainwrapper Aptos converter creation to pass the new option when
mcmsTypeindicates Curse. - Adjusted Aptos converter tests to use the new option-based constructor.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sdk/aptos/timelock_converter_test.go | Updates constructor test coverage to use the new option-based API for Curse. |
| sdk/aptos/timelock_converter.go | Introduces TimelockConverterOption / WithMCMSType and routes encoder binding based on selected MCMS type. |
| chainwrappers/converters.go | Switches Aptos converter construction to pass WithMCMSType(MCMSTypeCurse) when indicated by metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func NewTimelockConverter(opts ...TimelockConverterOption) *TimelockConverter { | ||
| converterOptions := timelockConverterOptions{mcmsType: MCMSTypeRegular} | ||
| for _, opt := range opts { | ||
| opt(&converterOptions) | ||
| } | ||
|
|
||
| if converterOptions.mcmsType == MCMSTypeCurse { | ||
| return &TimelockConverter{ | ||
| encoderFn: func(address aptos.AccountAddress, client aptos.AptosRpcClient) timelockEncoder { | ||
| return curse_mcms.Bind(address, client).CurseMCMS().Encoder() | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
This change removes the exported NewCurseTimelockConverter constructor entirely. If sdk/aptos is consumed as a library, that’s a breaking API change; consider keeping NewCurseTimelockConverter as a thin wrapper around NewTimelockConverter(WithMCMSType(MCMSTypeCurse)) (optionally marked deprecated) to preserve backward compatibility.
| }, | ||
| type TimelockConverterOption func(*timelockConverterOptions) | ||
|
|
||
| type timelockConverterOptions struct{ |
There was a problem hiding this comment.
type timelockConverterOptions struct{ is not gofmt-formatted (missing space before {). CI appears to enforce gofmt/goimports (see .golangci.yml), so this will likely fail formatting checks. Run gofmt on this file (or adjust the declaration) before merging.
| type timelockConverterOptions struct{ | |
| type timelockConverterOptions struct { |
gustavogama-cll
force-pushed
the
ggama/chore/refactor-aptos-curse-converters-constructor-as-func-option
branch
from
e86d0c0 to
fc0d2f7
Compare
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.