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

[SC] Embedded Smart Contracts #534

Open
wants to merge 35 commits into
base: infracontracts
Choose a base branch
from

Conversation

quantumagi
Copy link
Contributor

@quantumagi quantumagi commented Apr 27, 2021

See https://app.clickup.com/t/6tzz8e.

This PR:

  • Implements embedded smart contracts on both Strax and Cirrus to support the Authentication and MultiSig contracts.

The Authentication contract:

  • Allows maintaining multiple groups of signatories including the "main" group which is the (Stratis) primary signatories.
  • The default list of signatories is defined on the network class, in the form of specified wallet addresses.
  • All groups, including "main", are maintained by a quorum of the "main" group.

The MultiSig contract:

  • Can be called to modify the multisig federation when provided with a quorum of signatures from the "main" group.
  • Signatures are obtained using the "signmessage" API. The message depends on the method (+arguments) being called.
  • Signature verification is delegated to the Authentication contract.

Embedded (or bundled) smart contracts behave exactly the same as normal smart contracts in all respects, except:

  • The SC code is not rewritten (and cached) as would normally be required to monitor memory and gas consumption.
  • Implicit gas spending is thereby disabled but gas can still be spent explicitly by the embedded contract as and if required.
  • These contracts have no deployment step and as such need to maintain an "Initialized" flag to determine whether their state had been initialized yet.
  • The SC constructor is called before each method call to give the contract an opportunity to initialize its state, if it had not already done so, and is also a great opportunity for the VM to pass in any service references required by the contract.
  • Code has no imposed restrictions internally, and has access to all services. However interfacing standards are maintained.

Includes the MultiSig contract with test cases and shows how it authenticates by delegating to the Authentication contract.

You can get a birds-eye view by stepping through theVM_ExecuteEmbeddedContract_WithParameters test case.

The most notable changes are found in the EmbeddedContractMachine.cs class and in the two contracts being presented.

@quantumagi quantumagi requested a review from rowandh April 27, 2021 06:54
@quantumagi quantumagi marked this pull request as ready for review May 3, 2021 07:43
@quantumagi quantumagi changed the title [Infra] Add Multisig contract [Infra] Embedded Smart Contracts May 3, 2021
@quantumagi quantumagi requested a review from YakupIpek May 3, 2021 07:45
@quantumagi quantumagi requested a review from noescape00 May 10, 2021 04:55
@quantumagi quantumagi requested a review from fassadlr June 10, 2021 03:57
@quantumagi quantumagi requested a review from zeptin June 10, 2021 06:36
@quantumagi quantumagi changed the title [Infra] Embedded Smart Contracts [SC] Embedded Smart Contracts Jun 11, 2021
this.authentication = new Authentication(GetState(state, persistenceStrategy, EmbeddedContractAddress.Create(typeof(Authentication), 1)), network);

// Exit if already initialized.
if (this.Initialized)
Copy link
Contributor

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?

Copy link
Contributor Author

@quantumagi quantumagi Jun 14, 2021

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rowandh Wdyt about this ?

Copy link
Contributor Author

@quantumagi quantumagi Nov 2, 2021

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.

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);
Copy link
Member

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.

Copy link
Contributor Author

@quantumagi quantumagi Jun 17, 2021

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? :)

Copy link
Member

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.

Copy link
Contributor Author

@quantumagi quantumagi Jun 17, 2021

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants