fix: make predecessors parameter optional (#297)

Predecessors can be optional and user should not be forced to provide an
empty value if not needed on the API for `NewProposal` and
`NewTimelockProposal`. This commit introduces a functional option for
`predecessors` and making that config optional, so the api is cleaner.

This change was created after a brief discussion with James.
This is a breaking change, so open to recommendations if we want to do
it or not ( do we care enough?) but better do it now than later before
we have more users.

This is our official first breaking change, this will be a major version
bump.

Author: graham-chainlink

.changeset/hip-adults-sniff.md

@@ -0,0 +1,5 @@
+---
+"@smartcontractkit/mcms": major
+---
+
+fix: make predecessors parameter optional on NewProposal and NewTimelockProposal

docs/usage/building-proposals.md

@@ -36,7 +36,7 @@ func main() {
   defer file.Close()
 
   // Create the proposal from the JSON data
-  proposal, err := mcms.NewProposal(file, []io.Reader{})
+  proposal, err := mcms.NewProposal(file)
   if err != nil {
     log.Fatalf("Error creating proposal: %v", err)
   }
@@ -49,7 +49,9 @@ For the JSON structure of the proposal please check the [MCMS Proposal Format Do
 
 ### Build Proposal Given Staged but Non-Executed Predecessor Proposals
 
-In scenarios where a proposal is generated with the assumption that multiple proposals are executed beforehand, you can enable proposals to be signed in parallel with a pre-determined execution order. This can be achieved by passing a list of files as the second argument in the Proposal constructor, as shown below:
+In scenarios where a proposal is generated with the assumption that multiple proposals are executed beforehand, 
+you can enable proposals to be signed in parallel with a pre-determined execution order. This can be achieved
+by passing a list of files using the `WithPredecessors` functional option, as shown below:
 
 ```go
 package main
@@ -78,7 +80,7 @@ func main() {
   defer preFile.Close()
 
   // Create the proposal from the JSON data
-  proposal, err := mcms.NewProposal(file, []io.Reader{preFile})
+  proposal, err := mcms.NewProposal(file, mcms.WithPredecessors([]io.Reader{preFile}))
   if err != nil {
     log.Fatalf("Error creating proposal: %v", err)
   }

e2e/ledger/ledger_test.go

@@ -5,7 +5,6 @@ package ledger
 
 import (
 	"context"
-	"io"
 	"log"
 	"math/big"
 	"os"
@@ -204,7 +203,7 @@ func (s *ManualLedgerSigningTestSuite) TestManualLedgerSigning() {
 	}(file)
 	s.Require().NoError(err)
 
-	proposal, err := mcms.NewProposal(file, []io.Reader{})
+	proposal, err := mcms.NewProposal(file)
 	s.Require().NoError(err, "Failed to parse proposal")
 	s.T().Log("Proposal loaded successfully.")
 	proposal.ChainMetadata[s.chainSelectorEVM] = types.ChainMetadata{

e2e/tests/evm/signing.go

@@ -49,7 +49,7 @@ func (s *SigningTestSuite) TestReadAndSign() {
 		}
 	}(file)
 	s.Require().NoError(err)
-	proposal, err := mcms.NewProposal(file, []io.Reader{})
+	proposal, err := mcms.NewProposal(file)
 	s.Require().NoError(err)
 	s.Require().NotNil(proposal)
 	inspectors := map[mcmtypes.ChainSelector]sdk.Inspector{
@@ -81,7 +81,7 @@ func (s *SigningTestSuite) TestReadAndSign() {
 	_, err = tmpFile.Seek(0, io.SeekStart)
 	s.Require().NoError(err, "Failed to reset file pointer to the start")
 
-	writtenProposal, err := mcms.NewProposal(tmpFile, []io.Reader{})
+	writtenProposal, err := mcms.NewProposal(tmpFile)
 	s.Require().NoError(err)
 
 	// Validate the appended signature

proposal.go

@@ -44,7 +44,7 @@ func LoadProposal(proposalType types.ProposalKind, filePath string) (ProposalInt
 		// Ensure the file is closed when done
 		defer file.Close()
 
-		return NewProposal(file, []io.Reader{}) // TODO: inject predecessors
+		return NewProposal(file)
 	case types.KindTimelockProposal:
 		// Open the file
 		file, err := os.Open(filePath)
@@ -55,7 +55,7 @@ func LoadProposal(proposalType types.ProposalKind, filePath string) (ProposalInt
 		// Ensure the file is closed when done
 		defer file.Close()
 
-		return NewTimelockProposal(file, []io.Reader{}) // TODO: inject predecessors
+		return NewTimelockProposal(file)
 	default:
 		return nil, errors.New("unknown proposal type")
 	}
@@ -107,18 +107,38 @@ type Proposal struct {
 
 var _ ProposalInterface = (*Proposal)(nil)
 
-// NewProposal unmarshals data from the reader to JSON and returns a new Proposal.
-// The predecessors parameter is a list of readers that contain the predecessors
-// for the proposal for configuring operations counts, which makes the following
+type ProposalOption func(*proposalOptions)
+
+type proposalOptions struct {
+	predecessors []io.Reader
+}
+
+// WithPredecessors is an option that allows the user to specify a list of
+// that contain the predecessors for the proposal for configuring operations counts, which makes the following
 // assumptions:
 //   - The order of the predecessors array is the order in which the proposals are
 //     intended to be executed.
 //   - The op counts for the first proposal are meant to be the starting op for the
 //     full set of proposals.
 //   - The op counts for all other proposals except the first are ignored
 //   - all proposals are configured correctly and need no additional modifications
-func NewProposal(reader io.Reader, predecessors []io.Reader) (*Proposal, error) {
-	return newProposal[*Proposal](reader, predecessors)
+func WithPredecessors(predecessors []io.Reader) ProposalOption {
+	return func(opts *proposalOptions) {
+		opts.predecessors = predecessors
+		if opts.predecessors == nil {
+			opts.predecessors = []io.Reader{}
+		}
+	}
+}
+
+// NewProposal unmarshals data from the reader to JSON and returns a new Proposal.
+func NewProposal(reader io.Reader, opts ...ProposalOption) (*Proposal, error) {
+	options := &proposalOptions{}
+	for _, opt := range opts {
+		opt(options)
+	}
+
+	return newProposal[*Proposal](reader, options.predecessors)
 }
 
 // WriteProposal marshals the proposal to JSON and writes it to the provided writer.

proposal_test.go

@@ -214,7 +214,7 @@ func Test_NewProposal(t *testing.T) {
 				givePredecessors = append(givePredecessors, strings.NewReader(p))
 			}
 
-			fileProposal, err := NewProposal(give, givePredecessors)
+			fileProposal, err := NewProposal(give, WithPredecessors(givePredecessors))
 
 			if tt.wantErr != "" {
 				require.EqualError(t, err, tt.wantErr)

timelock_proposal.go

@@ -31,17 +31,13 @@ type TimelockProposal struct {
 var _ ProposalInterface = (*TimelockProposal)(nil)
 
 // NewTimelockProposal unmarshal data from the reader to JSON and returns a new TimelockProposal.
-// The predecessors parameter is a list of readers that contain the predecessors
-// for the proposal for configuring operations counts, which makes the following
-// assumptions:
-//   - The order of the predecessors array is the order in which the proposals are
-//     intended to be executed.
-//   - The op counts for the first proposal are meant to be the starting op for the
-//     full set of proposals.
-//   - The op counts for all other proposals except the first are ignored
-//   - all proposals are configured correctly and need no additional modifications
-func NewTimelockProposal(r io.Reader, predecessors []io.Reader) (*TimelockProposal, error) {
-	return newProposal[*TimelockProposal](r, predecessors)
+func NewTimelockProposal(r io.Reader, opts ...ProposalOption) (*TimelockProposal, error) {
+	options := &proposalOptions{}
+	for _, opt := range opts {
+		opt(options)
+	}
+
+	return newProposal[*TimelockProposal](r, options.predecessors)
 }
 
 func WriteTimelockProposal(w io.Writer, p *TimelockProposal) error {

timelock_proposal_test.go

@@ -285,7 +285,7 @@ func Test_NewTimelockProposal(t *testing.T) {
 				givePredecessors = append(givePredecessors, strings.NewReader(p))
 			}
 
-			got, err := NewTimelockProposal(give, givePredecessors)
+			got, err := NewTimelockProposal(give, WithPredecessors(givePredecessors))
 
 			if tt.wantErr != "" {
 				require.EqualError(t, err, tt.wantErr)