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

Get rid of Transaction<T>.UpdatedAddresses and “rehearsal mode”, and more #368

Closed
dahlia opened this issue Jul 24, 2019 · 7 comments · Fixed by #3122
Closed

Get rid of Transaction<T>.UpdatedAddresses and “rehearsal mode”, and more #368

dahlia opened this issue Jul 24, 2019 · 7 comments · Fixed by #3122
Assignees
Labels
storage Related to storage (Libplanet.Store) suggestion Suggestion or feature request

Comments

@dahlia
Copy link
Contributor

dahlia commented Jul 24, 2019

Historically, at the very beginning, we had Transaction.SenderTransaction.Recipient model, which was made after Bitcoin's or Ethereum's cryptocurrency model. In 0.2 release, we became to need “multiple recipients” because Libplanet is not for cryptocurrencies but games, so we renamed the terminology to Transaction.SignerTransaction.UpdatedAddresses (#121). However, it's still unhandy to manually specify the list of state addresses that every action in a transaction will mutate, so we had “rehearsal mode” as well in the end (#131), which turns out overkill so that everybody (including me who invented it) dislikes. 🤷🏻‍♂️

In Ethereum, the concept of states are associated with accounts, so keys of the global state map are account addresses. This model works well with the concept of smart contract, which is a kind of accounts hence has an address for each in Ethereum; every smart contract has its own code and state, and there's kind of access control, mutating a contract's state can be done by the code of the same contract (though anyone can read any contract's state). It's basically cryptocurrency and assets, and if you want users to be able to upload some code (i.e., smart contracts), code must not steel others' private assets without owners' authorization.

On the other hand, Libplanet is for games and has no concept of smarts contracts, but follows application-specific blockchain instead. Executed code in the network is part of consensus, and code with no consensus is never executed in the network. Hence we don't need access control unlike Ethereum.

So here's a conclusion: we don't need Transaction.UpdatedAddresses property at all, hance, no “rehearsal mode” either.

Although it's further than the topic, I've recently started to believe keys of the global state map don't have to be (and shouldn't be) addresses, but arbitrary strings instead, which is more easier to understand to existing programmers who are familiar to Redis or memcached.

Any opinions?

@dahlia dahlia added the suggestion Suggestion or feature request label Jul 24, 2019
@longfin
Copy link
Member

longfin commented Jul 24, 2019

That sounds good.

@stale
Copy link

stale bot commented Oct 30, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Oct 30, 2019
@stale stale bot closed this as completed Nov 6, 2019
@limebell limebell reopened this Nov 7, 2019
@stale stale bot removed the stale The issue is stale label Nov 7, 2019
@stale
Copy link

stale bot commented Jan 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Jan 6, 2020
@dahlia dahlia removed the stale The issue is stale label Jan 21, 2020
@dahlia dahlia self-assigned this Jan 21, 2020
@dahlia dahlia added the storage Related to storage (Libplanet.Store) label Jan 21, 2020
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 21, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 22, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 23, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 23, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 23, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 28, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
dahlia added a commit to dahlia/libplanet that referenced this issue Jan 28, 2020
It's the first step to refer to states by arbitrary string keys
instead of Addresses, as I suggested in the issue:

planetarium#368

> Although it's further than the topic, I've recently started to
> believe keys of the global state map don't have to be (and shouldn't
> be) addresses, but arbitrary strings instead, which is more easier to
> understand to existing programmers who are familiar to Redis
> or memcached.
@stale
Copy link

stale bot commented Mar 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Mar 21, 2020
@dahlia dahlia removed the stale The issue is stale label Jul 14, 2020
@stale
Copy link

stale bot commented Sep 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Sep 12, 2020
limebell pushed a commit to limebell/libplanet that referenced this issue Jul 7, 2021
@dahlia dahlia changed the title Get rid of Transaction.UpdatedAddresses and “rehearsal mode”, and more Get rid of Transaction.UpdatedAddresses and “rehearsal mode”, and more May 23, 2022
@stale stale bot removed the stale The issue is stale label May 23, 2022
@dahlia
Copy link
Contributor Author

dahlia commented May 23, 2022

We will never get rid of Transaction<T>.UpdatedAddresses field, because it already affected already signed transactions. If we remove the field their signatures and IDs would change too.

However, we still can remove the guarantees that each transaction can change only the states listed in its UpdateAddresses at most, (and indeed such guarantee had been silently a dead letter since #1389,) and further get rid of the “rehearsal mode” to automatically list UpdatedAddresses as well.

I'd like to suggest to change the meaning of UpdatedAddresses to the hint for lightweight (dumb) front-ends like Libplanet.Explorer, instead of the strong guarantee.

@dahlia dahlia changed the title Get rid of Transaction.UpdatedAddresses and “rehearsal mode”, and more Get rid of Transaction<T>.UpdatedAddresses and “rehearsal mode”, and more May 23, 2022
@stale
Copy link

stale bot commented Jul 31, 2022

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
storage Related to storage (Libplanet.Store) suggestion Suggestion or feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants