static mixin (protocol level) #30

Merged

wowario merged 1 commits from mixin into release-v0.2.1.0

wowario commented (Migrated from github.com)

Owner

After hard fork version 9, all transactions must have the same ring size.

stoffu (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

stoffu (Migrated from github.com) commented

Owner

This seems to overwrite the variable mixin as calculated above, making the if section below a no-op. Or am I missing something?

wowario (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

wowario (Migrated from github.com) commented

Owner

yes, it overwrites the variable mixin (which is set to max) to make sure mixin and min_mixin always equal DEFAULT_MIXIN after v9. I could separate the section to make it cleaner.

stoffu (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

stoffu (Migrated from github.com) commented

Owner

I don't get it - isn't the idea to reject txes whose mixins are different from the default mixin? Currently it doesn't do so.

wowario (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

wowario (Migrated from github.com) commented

Owner

Currently, users can pick any mixin above the min_mixin. After v9, users can only pick the default mixin.

wowario (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

wowario (Migrated from github.com) commented

Owner

would this make more sense?: 02ca1bc7b8

stoffu (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

stoffu (Migrated from github.com) commented

Owner

No, it doesn't make sense. This code is for verifying the validity of a given transaction, and the transaction's mixin value is obtained in the above loop for (const auto& txin : tx.vin) {...}. Then the obtained mixin variable is ignored and overwritten by const size_t mixin = DEFAULT_MIXIN;, so this code will accept txes with any mixin. Or am I reading anything wrong?

wowario (Migrated from github.com) reviewed

src/cryptonote_core/blockchain.cpp

wowario (Migrated from github.com) commented

Owner

Ah, okay. I see what you mean. Let me try to express it a different way.

The pull request has been merged as 0548011ca5.

Sign in to join this conversation.

No reviewers

No Label

No Milestone

No Assignees

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: wownero/wownero#30