search

dfinity / cycles-wallet

DFINITY Cycles Wallet
Apache License 2.0
55 stars 30 forks source link

feat: Track canisters and per-canister events #113

Closed adamspofford-dfinity closed 2 years ago

adamspofford-dfinity commented 2 years ago

Addresses SDK-231.

Currently, the cycles wallet records a unified list of events, and does not remember what canisters it created. This pull makes it so that it does remember its child canisters, and records each child-canister-related event separately by canister. The old collections will still be around for compatibility and easy filtering; the two will just be kept in sync. An optional parameter is also added to wallet_receive for an optional transaction memo.

adamspofford-dfinity commented 2 years ago

The term "Child" seems to be more about data being stored in a data structure, rather than being a fitting metaphor for the data that we're storing canisters that the wallet knows about and is a controller of.

@ericswanson-dfinity I was referring to them as child canisters, with the wallet being the parent canister. Hash maps did not enter the mind. Is this not a good term?

ericswanson-dfinity commented 2 years ago

The term "Child" seems to be more about data being stored in a data structure, rather than being a fitting metaphor for the data that we're storing canisters that the wallet knows about and is a controller of.

@ericswanson-dfinity I was referring to them as child canisters, with the wallet being the parent canister. Hash maps did not enter the mind. Is this not a good term?

Right, since there isn't a parent-child relationship between the wallet canister and the canisters that are created through it (and especially now that it's just 1 controller of many), I think we should try to come up with a different term to use in the wallet interface. I don't have any great answers to what that might be, but in the context of the wallet interface, it could even be something generic like list_canisters instead of list_children and so forth.

adamspofford-dfinity commented 2 years ago

@chenyan-dfinity Can you validate (or invalidate) what I'm saying about subtyping and reliability? Since the other guys are out and you would know if I'm right or not.

chenyan-dfinity commented 2 years ago

Yep, I can confirm the candid changes are backward-compatible. But I agree with @ericswanson-dfinity that we need to add e2e test for upgrade, so that we know for sure, data is not lost on upgrade.