callAPI has a side-effect which overwrites gas settings. When client commands are invoked in for loops, this clobbers user preferences.

[jshufro]

Several rocketpool-cli commands precede calling the API with a user-customizable gas setting update, ie,

        // Assign max fees
        err = gas.AssignMaxFeeAndLimit(gasInfo, rp, c.Bool("yes"))
        if err != nil {
                return err
        }

This function checks config, cli flags, and ultimately prompts for gas selection on stdin if none of the preceding settings exist. Subsequently, it modifies the global state of the Client instance:

// Get the gas fees
func (c *Client) AssignGasSettings(maxFee float64, maxPrioFee float64, gasLimit uint64) {
        c.maxFee = maxFee
        c.maxPrioFee = maxPrioFee
        c.gasLimit = gasLimit
}

Next, the rocketpool-cli commands typically execute a for loop, inside which the API is ultimately called after one level of indirection... ie stakeMinipools in rocketpool-cli/minipool/stake.go calls

        // Assign max fees
        err = gas.AssignMaxFeeAndLimit(gasInfo, rp, c.Bool("yes"))
        if err != nil {
                return err
        }
        ...
        // Stake minipools
        for _, minipool := range selectedMinipools {
                response, err := rp.StakeMinipool(minipool.Address)
                ...
        }

but rp.StakeMinipool is a light wrapper around shared/services/rocketpool/client.go's callAPI function which contains

        // Reset the gas settings after the call
        c.maxFee = c.originalMaxFee
        c.maxPrioFee = c.originalMaxPrioFee
        c.gasLimit = c.originalGasLimit

as a clean-up. This means that the first entry in the for loop passes the correct gas settings to the api, but the subsequent entries do not respect the user settings.

I believe the ideal solution is to vectorize functions like StakeMinipool and extend callAPI to allow the invoker to trigger the cleanup, instead of triggering it after every API call, or to make the Client interface less stateful, forcing its users to pass gas settings in on every invocation

[jshufro]

All functions which invoke callAPI from a looping context and require gas:

• ClaimFromLot
• RecoverUnclaimedRPLFromLot
• RefundMinipool
• StakeMinipool
• DissolveMinipool
• CloseMinipool
• DelegateUpgradeMinipool
• DelegateRollbackMinipool
• SetUseLatestDelegateMinipool
• ExecuteTNDAOProposal

[0xfornax]

Merged.