-
Notifications
You must be signed in to change notification settings - Fork 276
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
fix: flatten asset definitions and assets in data model #4792
Conversation
08fac91
to
132e1d0
Compare
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.
you have to fix this:
- on
Unregister<Account>
remove all entries fromWorld::assets
- on
Unregister<Domain>
remove all entries fromWorld::asset_definitions
90ea924
to
682d6f5
Compare
Torii tests need to be updated, otherwise looks good |
2381938
to
0a3f986
Compare
aefab6d
to
b83b5b1
Compare
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.
Let me check smt.
EDIT:
As i said in my previous comment AssetId
s are ordered by AssetDefinitionId
first, so current usage of AssetByAccountBounds
is not correct, because all assets for AccountId
are not in the same adjacent to each other.
#[test]
fn asset_account_range() {
let domain_id: DomainId = "wonderland".parse().unwrap();
let account_id = gen_account_in("wonderland").0;
let accounts = [
account_id.clone(),
account_id.clone(),
gen_account_in("a").0,
gen_account_in("b").0,
gen_account_in("z").0,
gen_account_in("z").0,
];
let asset_definitions = [
AssetDefinitionId::new(domain_id.clone(), "a".parse().unwrap()),
AssetDefinitionId::new(domain_id.clone(), "f".parse().unwrap()),
AssetDefinitionId::new(domain_id.clone(), "b".parse().unwrap()),
AssetDefinitionId::new(domain_id.clone(), "c".parse().unwrap()),
AssetDefinitionId::new(domain_id.clone(), "d".parse().unwrap()),
AssetDefinitionId::new(domain_id.clone(), "e".parse().unwrap()),
];
let assets = accounts
.into_iter()
.zip(asset_definitions)
.map(|(account, asset_definiton)| AssetId::new(asset_definiton, account))
.map(|asset| (asset, ()));
let map = Storage::from_iter(assets);
let view = map.view();
let range = view.range(AssetByAccountBounds::new(&account_id));
assert_eq!(range.count(), 2);
}
b83b5b1
to
6a1be72
Compare
I swapped the internal order of AssetId to make it sort correctly. Api remains the same. |
6a1be72
to
586fd61
Compare
Signed-off-by: Sam H. Smith <[email protected]>
586fd61
to
98f79d7
Compare
Description
Linked issue
Closes #{issue_number}
Benefits
Checklist
CONTRIBUTING.md