Skip to content
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

Add StakeTransaction #2401

Open
wants to merge 3 commits into
base: 2.0-base
Choose a base branch
from
Open

Conversation

Tommytrg
Copy link
Member

@Tommytrg Tommytrg commented Oct 9, 2023

Validations

  • Transaction

    • It must stake the minimum allowed (10_000_000 nanowits).
    • It must have at least one input.
    • It must have a valid signature.
  • Block

    • A block must not contain more than one stake transaction from the same operator.
    • A block must not exceed the maximum weight allowed per block (10_000 weight units).

@Tommytrg Tommytrg marked this pull request as draft October 9, 2023 16:33
@Tommytrg Tommytrg force-pushed the feat/stake-tx branch 8 times, most recently from c64ab6d to e724019 Compare October 16, 2023 14:17
@Tommytrg Tommytrg marked this pull request as ready for review October 18, 2023 11:26
data_structures/src/chain/mod.rs Outdated Show resolved Hide resolved
data_structures/src/error.rs Outdated Show resolved Hide resolved
data_structures/src/error.rs Outdated Show resolved Hide resolved
data_structures/src/error.rs Outdated Show resolved Hide resolved
data_structures/src/error.rs Outdated Show resolved Hide resolved
validations/src/validations.rs Show resolved Hide resolved
validations/src/validations.rs Outdated Show resolved Hide resolved
validations/src/validations.rs Outdated Show resolved Hide resolved
Comment on lines +1823 to +1813
// TODO: Move validations to a visitor
// // Execute visitor
// if let Some(visitor) = &mut visitor {
// let transaction = Transaction::ValueTransfer(transaction.clone());
// visitor.visit(&(transaction, fee, weight));
// }
Copy link
Member

Choose a reason for hiding this comment

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

👀

Comment on lines +190 to +191
// #[serde(rename = "stake")]
// Stake(StakeData),
Copy link
Member

Choose a reason for hiding this comment

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

Uncomment?

@Tommytrg Tommytrg force-pushed the feat/stake-tx branch 3 times, most recently from 2cc250d to a21c0a8 Compare October 19, 2023 15:35
Comment on lines +2258 to +2263
/// # use witnet_data_structures::chain::{TransactionsPool, Hash};
/// # use witnet_data_structures::transaction::{Transaction, StakeTransaction};
Copy link
Member

Choose a reason for hiding this comment

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

Why are these comented out?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because it's also comented in the other examples

data_structures/src/chain/mod.rs Outdated Show resolved Hide resolved
data_structures/src/chain/mod.rs Outdated Show resolved Hide resolved
data_structures/src/chain/mod.rs Outdated Show resolved Hide resolved
data_structures/src/chain/mod.rs Outdated Show resolved Hide resolved
validations/src/validations.rs Outdated Show resolved Hide resolved
Comment on lines +112 to +124
let out_value = &st_tx.body.output.value
- &st_tx
.body
.change
.clone()
.unwrap_or(Default::default())
.value;

if out_value > in_value {
Err(TransactionError::NegativeFee.into())
} else {
Ok(in_value - out_value)
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't get this logic. Why subtracting the change? The change is also part of the transaction value.

failure::Error,
> {
// Check that the amount of coins to stake is equal or greater than the minimum allowed
if st_tx.body.output.value < MIN_STAKE_NANOWITS {
Copy link
Member

Choose a reason for hiding this comment

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

See my former comment on consistency of ? vs. return syntax inside the same function.

validations/src/validations.rs Outdated Show resolved Hide resolved
validations/src/validations.rs Outdated Show resolved Hide resolved
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