[PoX-4] Match function signature for extend methods

[janniks]

Currently (next branch at time of writing), the methods: stack-extend and delegate-stack-extend have slightly differing signatures, even though they overlap in arguments a lot.
Additionally, all other signer-key accepting stacking methods have the signer-key as their last argument.

stacks-core/stackslib/src/chainstate/stacks/boot/pox-4.clar

Lines 963 to 965 in 3e21883

	(define-public (stack-extend (extend-count uint)
	(pox-addr { version: (buff 1), hashbytes: (buff 32) })
	(signer-key (buff 33)))

stacks-core/stackslib/src/chainstate/stacks/boot/pox-4.clar

Lines 1138 to 1142 in 3e21883

	(define-public (delegate-stack-extend
	(stacker principal)
	(pox-addr { version: (buff 1), hashbytes: (buff 32) })
	(signer-key (buff 33))
	(extend-count uint))

Might this be a good time to change the delegate-stack-extend signature (argument ordering)? Since, with the pox-4 update users will have to revisit any breaking code already.

to e.g.

(define-public (delegate-stack-extend
                    (stacker principal)
                    (extend-count uint)
                    (pox-addr { version: (buff 1), hashbytes: (buff 32) })
                    (signer-key (buff 33)))

CC @friedger

Haven't found anything exact on this in the SIPs, but I might be missing something.

[friedger]

The proposed change makes sense to me. Let's do it. @setzeus

[setzeus]

ACK. I don't see any reason we can't do this & indeed remember coming across the different order, thanks for the note @janniks.

Edit*** discussed w/ Jude below.

[jcnelson]

Hi, we don't know who out there would be affected by a change to the public API signature, so we can't assess what the fallout would be of changing the arguments. If you feel really strongly about the aesthetics, then please add a separate public function that just calls this one (but don't change the existing one).

[janniks]

This would only be changing the signatures, which are already being changed from pox-3 → pox-4 (i.e. adding the signer-key argument), so I'm assuming nobody depends on the public API signature yet. And folks who update their code from pox-3 to pox-4, could switch two args (in addition to adding the signer-key arg (which they likely are already looking up how to do).

[jcnelson]

It is a lot easier to change the contract than the signatures, especially if the former is already expected but the latter isn't

…

On Mon, Jan 8, 2024, 11:41 AM janniks ***@***.***> wrote: This would only be changing the signatures, which are already being changed from pox-3 → pox-4 (i.e. adding the signer-key argument) — Reply to this email directly, view it on GitHub <#4230 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQJK6QPNJGUXFHMYH7D7DYNQOSNAVCNFSM6AAAAABBNGFOTCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBRGQ2DENBQGE> . You are receiving this because you commented.Message ID: ***@***.***>

[friedger]

Developers/users need to change the signature already when switching to pox-4. The api is not yet released in one way or the other.

The change is about quality of code/mental load, not aesthetics

[janniks]

Partially closed by #4279 -- I'm fine to close, at least signer-key will be consistently last. The other params I'm okay with keeping the same as pox-3 -- Feel free to reopen if someone feels strongly about the remaining param order.

[blockstack-devops]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.