Skip to content

sighash input commitments - WIP #1910

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ImplOfAnImpl
Copy link
Contributor

No description provided.

/// `ProduceBlockFromStake`. If the former is consumed, then the pool hasn't staked yet,
/// so committing to `TxOutput` is enough, which is covered by the `Utxo` case.
#[codec(index = 2)]
ProduceBlockFromStakeUtxo { staker_balance: Amount },
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd say that TxOutput should be here anyway. Imo it's not redundant and is the rule of spending utxos - you have to provide spent output. ProduceBlockFromStake should just have additional requirement (staker's balance).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[discussed] it's probably not a rule when spending utxos in general (e.g. we do commit to TxInput and, therefore, UtxoOutPoint anyway). But in this particular case it's better to commit to the pool id as well, which is in TxOutput. So I'll put TxOutput here too.

/// For FillOrder we want to commit to the initial balances, which are used to calculate
/// the exchange ratio in orders V1.
///
/// Note that we do NOT want to commit to the current balances here, because this would make
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd not only "make them non-commutative" it basically would be incorrect according to the latest price calculation rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

[discussed] the commitment won't affect price calculation rules, so the comment is ok as is.

Base automatically changed from forbid_staking_destination_change to master April 21, 2025 16:10
Sign up for free to 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.

2 participants