Skip to content

docs: document get-pox-addr and the signer-calldata a manager must accept - #1883

Open
heidar-stacks wants to merge 1 commit into
stacks-network:masterfrom
heidar-stacks:docs/signer-manager-payout-election-requirements
Open

docs: document get-pox-addr and the signer-calldata a manager must accept#1883
heidar-stacks wants to merge 1 commit into
stacks-network:masterfrom
heidar-stacks:docs/signer-manager-payout-election-requirements

Conversation

@heidar-stacks

Copy link
Copy Markdown

Description

document get-pox-addr and the signer-calldata a manager must accept

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

At claim time `claim-staker-rewards` reads that entry back. With a record it calls `sbtc-withdrawal::initiate-withdrawal-request`, and without one it transfers sBTC directly.
At claim time `claim-staker-rewards` reads that entry back. With a record it calls `sbtc-withdrawal::initiate-withdrawal-request`, and without one it transfers sBTC directly. `max-fee` is the fee ceiling it passes to that withdrawal, not a fee the manager takes.

Clients read the same entry through `get-pox-addr`, which answers `none` for a staker who has not elected an address:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Clients read the same entry through `get-pox-addr`, which answers `none` for a staker who has not elected an address:
Clients read the same entry through `get-pox-addr`, which answers `none` for a staker who has not elected to enter an address:

```

At claim time `claim-staker-rewards` reads that entry back. With a record it calls `sbtc-withdrawal::initiate-withdrawal-request`, and without one it transfers sBTC directly.
At claim time `claim-staker-rewards` reads that entry back. With a record it calls `sbtc-withdrawal::initiate-withdrawal-request`, and without one it transfers sBTC directly. `max-fee` is the fee ceiling it passes to that withdrawal, not a fee the manager takes.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this to

Suggested change
At claim time `claim-staker-rewards` reads that entry back. With a record it calls `sbtc-withdrawal::initiate-withdrawal-request`, and without one it transfers sBTC directly. `max-fee` is the fee ceiling it passes to that withdrawal, not a fee the manager takes.
At claim time `claim-staker-rewards` reads that entry back. With a record it calls `sbtc-withdrawal::initiate-withdrawal-request`, and without one it transfers sBTC directly. `max-fee` is the fee ceiling it passes to the sbtc withdrawal smart contract.

The last bit about it not being a fee the manager takes is kind of tricky. The difference between the fee charged and the max-fee remains with the signer-manager, unless the withdrawal request fails.


* Both `none` and `(some buffer)` arrive, depending on which reward asset the staker chose. Requiring either one breaks the stakers who chose the other.
* Deserialize `hashbytes` as `(buff 32)`. A 20-byte value fits that type and a 32-byte value does not fit `(buff 20)`, and `from-consensus-buff?` answers a type mismatch with `none`, which surfaces as `ERR_INVALID_CALLDATA`.
* Accept the two-field `{ pox-addr, max-fee }` shape even if you add fields of your own, the way `fastpool-max500-signer-manager` accepts it alongside its own three-field tuple.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice, I didn't realize that Friedgar's contract accepts both forms. Nice job on him.


It returns `(optional { pox-addr: { version: (buff 1), hashbytes: (buff 32) }, max-fee: uint })`. This is how an app shows a staker where their rewards will land, and how it tells whether a manager keeps an election at all.

**What your manager has to accept**

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change the wording here. The signer manager doesn't really have to accept anything like what is described. It would be nice if the contracts followed these prescriptions, but we should explain why.

Comment on lines +85 to +91
```clarity
(define-read-only (get-pox-addr (staker principal))
(map-get? pox-addrs staker)
)
```

It returns `(optional { pox-addr: { version: (buff 1), hashbytes: (buff 32) }, max-fee: uint })`. This is how an app shows a staker where their rewards will land, and how it tells whether a manager keeps an election at all.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why mention this when below we show that some contracts don't have the function?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@heidar-stacks@CLAassistant@djordon-stacks