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

Content dir 2 issue 13 #278

Closed
wants to merge 26 commits into from
Closed

Content dir 2 issue 13 #278

wants to merge 26 commits into from

Conversation

iorveth
Copy link
Contributor

@iorveth iorveth commented Apr 13, 2020

Make vector properties updatable as vectors

Joystream/substrate-versioned-store-module#13

@iorveth iorveth marked this pull request as draft April 13, 2020 12:48
@iorveth iorveth changed the base branch from development to content_dir_2 April 13, 2020 12:49
@iorveth iorveth marked this pull request as ready for review April 15, 2020 23:09
@bedeho bedeho self-requested a review April 16, 2020 07:08
Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

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

My understanding of this solution is that it enforces the rule that each vector typed property value can only be mutated, in any way, at most once per entity per block.

Is this correct?

If so, then this is really overly restrictive solution to the problem. You should keep in mind that both Substrate and Ethereum use nonces to regulate race condition/replay attacks, and neither prevents a single account to make at most one transaction per block. Not only is it too restrictive, it actually does not even solve the full problem, because two competing transactions can still make a conflicting update, so long as it's across two blocks. What you want is to force each transaction to explicitly commit to known state of the vector.

Instead what you want is to introduce an explicit counter per such entity & value combination, and increment it each time there is a mutation operation. Whenever someone makes an extrinsic to make a vector mutation on an entity, they must provide the current counter value as a parameter, and the extrinsic is only accepted if it matches.

This approach allows someone to make multiple extrinsics in a row, with an increasing nonce value in each, where all can be accepted in the same block.

@iorveth iorveth requested a review from bedeho April 17, 2020 11:59
Copy link
Member

@bedeho bedeho left a comment

Choose a reason for hiding this comment

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

This was a good step in the right direction. I was, however, hoping you would see that the proper place for the nonce value is PropertyValue representation. So

pub enum PropertyValue {
...
    BoolVec(Vec<bool>, T::Nonce),
...
}

@bedeho bedeho mentioned this pull request Apr 19, 2020
45 tasks
@iorveth iorveth closed this May 6, 2020
@bedeho
Copy link
Member

bedeho commented May 6, 2020

Superceeded by #267

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