-
Notifications
You must be signed in to change notification settings - Fork 68
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?
Conversation
src/Stratis.Bitcoin.Features.SmartContracts/PoS/EmbeddedContractContainer.cs
Outdated
Show resolved
Hide resolved
this.authentication = new Authentication(GetState(state, persistenceStrategy, EmbeddedContractAddress.Create(typeof(Authentication), 1)), network); | ||
|
||
// Exit if already initialized. | ||
if (this.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.
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.
src/Stratis.Bitcoin.Features.SmartContracts/Embedded/Multisig.cs
Outdated
Show resolved
Hide resolved
src/Stratis.Bitcoin.Features.SmartContracts/Embedded/Multisig.cs
Outdated
Show resolved
Hide resolved
src/Stratis.Bitcoin.Features.SmartContracts/Embedded/Multisig.cs
Outdated
Show resolved
Hide resolved
src/Stratis.Bitcoin.Features.SmartContracts/Embedded/Multisig.cs
Outdated
Show resolved
Hide resolved
Assert(version == 1, "Only a version of 1 is supported."); | ||
|
||
this.version = version; | ||
this.authentication = new Authentication(GetState(state, persistenceStrategy, EmbeddedContractAddress.Create(typeof(Authentication), 1)), network); |
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.
See https://app.clickup.com/t/6tzz8e.
This PR:
Strax
andCirrus
to support theAuthentication
andMultiSig
contracts.The
Authentication
contract:The
MultiSig
contract:Authentication
contract.Embedded (or bundled) smart contracts behave exactly the same as normal smart contracts in all respects, except:
Includes the
MultiSig
contract with test cases and shows how it authenticates by delegating to theAuthentication
contract.You can get a birds-eye view by stepping through the
VM_ExecuteEmbeddedContract_WithParameters
test case.The most notable changes are found in the
EmbeddedContractMachine.cs
class and in the two contracts being presented.