-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validator bonds contract first version #6
Conversation
programs/validator-bonds/src/instructions/config/init_config.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/config/init_config.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/settlement/claim_settlement.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/settlement/claim_settlement.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/withdraw/withdraw_deposit.rs
Outdated
Show resolved
Hide resolved
Few notes to keep in mind as per our discussion:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@janlegner thanks a lot for the comments and the review.
Meanwhile I work on the base TS SDK calls and tests that I'm not putting to this PR and I add only changes particular to the contract itself.
- I renamed
deposit_bond
tofund_bond
(the way howfund_settlement
is named) and with thatwithdraw_deposit
toclaim_withdraw_request
(I want to have thewithdraw_request
to be used as term for init, cancel anda obtain action of the withdrawal
) - I found an runtime issue with deserialization of vote account that was adjusted
- I think to have parameterized the amount of minimum stake. It was 1 SOL as constant but with incoming changes to Solana it makes sense to me to be configurable in
Config
. - +1,
reset
stake means to activate the stake as well - I bit hesitate to change the term
withdrawer authority
towithdraw authority
. But I don't say I'm against. I would like just to double check. My initial attempt was to be consistent with stake account naming (https://github.com/solana-labs/solana/blob/v1.17.10/sdk/program/src/stake/state.rs#L279). I can see that the bot uses rather thewithdraw authority
term. So just to double check on intentions and naming terminology. - that part with lockups is right and I'm aware that SDK needs to manage it. On rechecking this I changed the way how the lockup is checked. Custodian handling removed. It complicates the thing, it could be added later.
- thanks for noting on the merging of the deactivated stake accounts. The
merge
permits only to merge when the staker authorities matches and when the delegation matches. I recheck the other commands and updated to be aware of the different stake accounts can be provided maliciously.
programs/validator-bonds/src/instructions/config/init_config.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/settlement/init_settlement.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/settlement/fund_settlement.rs
Outdated
Show resolved
Hide resolved
programs/validator-bonds/src/instructions/withdraw/withdraw_deposit.rs
Outdated
Show resolved
Hide resolved
My bad, I thought the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can merge this and continue work in consequent PRs
PR contains the code for the raw on-chain validator bonds program + some unit tests.
SDK, CLI, tests and functionality verification will be incoming in future code changes.
@janlegner this should be in a shape we talked about in our discussions and hopefully it stands for a functional design where only some details need to be adjusted.