Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[SC] Embedded Smart Contracts #534
base: infracontracts
Are you sure you want to change the base?
[SC] Embedded Smart Contracts #534
Changes from 33 commits
e67f72c
742ece4
8329e6a
d3196e3
341b4e1
f56193f
df97431
012caa2
a128ab0
4269f0f
067c428
5acab15
4e94126
0258c4a
1f7e952
1e3119d
0200759
9552bc4
2e1b1b0
c30382b
d41d3a4
c990047
82fcad9
dae9f00
27b5d4b
6b4e792
47c915f
a5369a3
547a329
4d7d3da
6e57908
61196ed
2323f22
c5ef05d
c056b46
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
It seems it makes more sense to me that Authentication deployed as another smart contract and MultiSig has to make external calls to that contract.
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.
Authentication is being deployed as a separate contract. In this case the contract can be called directly, which seems faster, so why not? :)
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.
This decision bring Initialized logic and it is not what expected in regular design for smart contracts.
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.
The "Initialized" flag is required due to the absence of a deployment step for embedded smart contracts.
#534 (comment)
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.
Can this happen?
I assume constructor is called only when contract is deployed, so how can it be initialized?
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.
The embedded contracts act a bit like transient singletons. When called via the
EmbeddedContractMachine
VM this constructor, rather than the default constructor (via reflection), is used to instantiate the contract to be called. Contract objects are not kept in memory as that may open the door to bad programming practices such as passing in-memory variables between calls.Initialized
keeps track of once-off state initialization to be performed by the contract.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.
@rowandh Wdyt about this ?
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.
I.e. for embedded contracts there is no enforced deployment step so the state must be initialized the first time it is used. The
Initialized
value itself is set and stored in the state to indicate that the state is initialized. Keep in mind that this constructor is called repeatedly as a result of a new contract object being used for each call. Such a new object may detect that the state is already initialized and skip the initialization.