Skip to content

Commit

Permalink
MESH-2006 multisig improvements part 2 (#1690)
Browse files Browse the repository at this point in the history 
* Refactor ProposalDetails and ProposalStatus.

* Convert multisig pallet to frame v2.

* Refactor Multisig events.

* Don't use Concat based hashing on the Proposal in storage map ProposalIds.

* Don't use Context::current_identity in multisig pallet.  Use the MS's linked DID or it's creator DID.

* Fix bridge controller's CreatorDID.

* Allow a primary key to use custom permissions when making a multisig a secondary key.

* Add tests for 'make_multisig_secondary'.

* Support adding/removing multiple MS signers.

* Emit one event when adding/removing many signers.

* Add execution reentry guard to multisig proposal execution.

* Fix benchmark for approve and create_proposal.

* Use bounded vec for multisig signers.

* Fix multisig unit tests.

* Make MaxSigners a constant in the metadata.
  • Loading branch information
@Neopallium
Neopallium committed Aug 13, 2024
1 parent 69539d0 commit fa4ccd8
Show file tree
Hide file tree
Showing 23 changed files with 1,395 additions and 880 deletions.
14 changes: 7 additions & 7 deletions integration/Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions integration/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ version = "0.1.0"
edition = "2021"

[patch.crates-io]
polymesh-api = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "asset_id_v7" }
polymesh-api-client = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "asset_id_v7" }
polymesh-api-client-extras = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "asset_id_v7" }
polymesh-api-tester = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "asset_id_v7" }
polymesh-api-codegen = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "asset_id_v7" }
polymesh-api-codegen-macro = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "asset_id_v7" }
polymesh-api = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "multisig_v7" }
polymesh-api-client = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "multisig_v7" }
polymesh-api-client-extras = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "multisig_v7" }
polymesh-api-tester = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "multisig_v7" }
polymesh-api-codegen = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "multisig_v7" }
polymesh-api-codegen-macro = { git = "https://github.com/PolymeshAssociation/polymesh-api", branch = "multisig_v7" }

[features]
default = []
Expand Down
6 changes: 6 additions & 0 deletions integration/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,12 @@ impl From<&PermissionsBuilder> for Permissions {
}
}

impl From<PermissionsBuilder> for Permissions {
fn from(builder: PermissionsBuilder) -> Self {
builder.build()
}
}

pub async fn get_auth_id(res: &mut TransactionResults) -> Result<Option<u64>> {
if let Some(events) = res.events().await? {
for rec in &events.0 {
Expand Down
154 changes: 105 additions & 49 deletions integration/tests/multisig_permissions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ async fn get_ms_proposal_id(res: &mut TransactionResults) -> Result<u64> {
let events = res.events().await?.expect("Failed to create MS proposal");
for rec in &events.0 {
match &rec.event {
RuntimeEvent::MultiSig(MultiSigEvent::ProposalAdded(_, _, id)) => {
return Ok(*id);
RuntimeEvent::MultiSig(MultiSigEvent::ProposalAdded { proposal_id, .. }) => {
return Ok(*proposal_id);
}
_ => (),
}
Expand All @@ -29,11 +29,8 @@ async fn ms_proposal_executed(res: &mut TransactionResults) -> Result<Option<boo
let events = res.events().await?.expect("Failed to approve MS proposal");
for rec in &events.0 {
match &rec.event {
RuntimeEvent::MultiSig(MultiSigEvent::ProposalExecuted(..)) => {
return Ok(Some(true));
}
RuntimeEvent::MultiSig(MultiSigEvent::ProposalFailedToExecute(..)) => {
return Ok(Some(false));
RuntimeEvent::MultiSig(MultiSigEvent::ProposalExecuted { result, .. }) => {
return Ok(Some(result.is_ok()));
}
_ => (),
}
Expand All @@ -42,28 +39,46 @@ async fn ms_proposal_executed(res: &mut TransactionResults) -> Result<Option<boo
}

pub struct MuliSigState {
creator: User,
api: Api,
creator: AccountSigner,
account: AccountId,
signers: Vec<AccountSigner>,
sigs_required: u64,
}

impl MuliSigState {
async fn create_and_join_creator(
tester: &mut PolymeshTester,
creator: &AccountSigner,
n_signers: usize,
sigs_required: u64,
as_primary: bool,
perms: Option<Permissions>,
) -> Result<MuliSigState> {
let mut ms = Self::create(tester, creator, n_signers, sigs_required).await?;
let mut res = if as_primary {
ms.make_primary().await?
} else {
ms.make_secondary(perms).await?
};
res.wait_in_block().await?;

Ok(ms)
}

async fn create(
tester: &mut PolymeshTester,
create_name: &str,
creator: &AccountSigner,
n_signers: usize,
sigs_required: u64,
join_did_as_primary: Option<bool>,
) -> Result<MuliSigState> {
let mut creator = creator.clone();
let mut signers = Vec::with_capacity(n_signers);
let signer_name_prefix = format!("{create_name}Signer");
let signer_name_prefix = format!("{:?}Signer", creator.account());
for idx in 0..n_signers {
signers.push(tester.new_signer_idx(&signer_name_prefix, idx)?);
}

let mut creator = tester.user(create_name).await?;

let mut res = tester
.api
.call()
Expand Down Expand Up @@ -106,35 +121,28 @@ impl MuliSigState {
}
}
}
RuntimeEvent::MultiSig(MultiSigEvent::MultiSigCreated(_, ms_account, ..)) => {
account = Some(*ms_account);
RuntimeEvent::MultiSig(MultiSigEvent::MultiSigCreated { multisig, .. }) => {
account = Some(*multisig);
}
_ => (),
}
}

let mut ms = MuliSigState {
for mut res in results {
res.wait_in_block().await?;
}

Ok(MuliSigState {
api: tester.api.clone(),
creator,
account: account.expect("Failed to get MS address"),
signers,
sigs_required,
};
match join_did_as_primary {
Some(true) => {
results.push(ms.make_primary().await?);
}
Some(false) => {
results.push(ms.make_secondary().await?);
}
None => (),
}

Ok(ms)
})
}

async fn make_primary(&mut self) -> Result<TransactionResults> {
let res = self
.creator
.api
.call()
.multi_sig()
Expand All @@ -144,29 +152,26 @@ impl MuliSigState {
Ok(res)
}

async fn make_secondary(&mut self) -> Result<TransactionResults> {
async fn make_secondary(
&mut self,
permissions: Option<Permissions>,
) -> Result<TransactionResults> {
let res = self
.creator
.api
.call()
.multi_sig()
.make_multisig_secondary(self.account.clone())?
.make_multisig_secondary(self.account.clone(), permissions)?
.submit_and_watch(&mut self.creator)
.await?;
Ok(res)
}

async fn create_proposal(&mut self, proposal: WrappedCall) -> Result<TransactionResults> {
let res = self
.creator
.api
.call()
.multi_sig()
.create_proposal(
self.account.clone(),
proposal.runtime_call().clone(),
None,
)?
.create_proposal(self.account.clone(), proposal.runtime_call().clone(), None)?
.submit_and_watch(&mut self.signers[0])
.await?;
Ok(res)
Expand All @@ -180,8 +185,7 @@ impl MuliSigState {
let id = get_ms_proposal_id(&mut res).await?;
let weight = Weight::from_parts(10_000_000_000, 0);
let approve_call =
self.creator
.api
self.api
.call()
.multi_sig()
.approve(self.account.clone(), id, weight)?;
Expand All @@ -204,13 +208,12 @@ impl MuliSigState {
}
}

async fn set_ms_key_permissions(
pub async fn set_ms_key_permissions(
&mut self,
permissions: impl Into<Permissions> + Send,
) -> Result<TransactionResults> {
let permissions = permissions.into();
let res = self
.creator
.api
.call()
.identity()
Expand All @@ -226,7 +229,6 @@ impl MuliSigState {
) -> Result<()> {
let permissions = permissions.into();
let record = self
.creator
.api
.query()
.identity()
Expand All @@ -248,10 +250,20 @@ async fn multisig_as_secondary_key_change_identity() -> Result<()> {
let users = tester
.users(&["MultiSigSecondaryDID1", "MultiSigSecondaryDID2"])
.await?;
let did1 = users[0].clone();
let mut did2 = users[1].clone();

let mut ms =
MuliSigState::create(&mut tester, "MultiSigSecondaryDID1", 3, 2, Some(false)).await?;
// Use the primary key of did1 to create a MS and join it do did1 as a secondary key.
let whole = PermissionsBuilder::whole();
let mut ms = MuliSigState::create_and_join_creator(
&mut tester,
&did1.primary_key,
3,
2,
false,
Some(whole.into()),
)
.await?;

// Create JoinIdentity auth for the MS to join DID2 with no-permissions.
let mut res = tester
Expand All @@ -269,10 +281,6 @@ async fn multisig_as_secondary_key_change_identity() -> Result<()> {
.await?
.expect("Missing JoinIdentity auth id");

// Change MS account's secondary key permissions.
let whole = PermissionsBuilder::whole();
ms.set_ms_key_permissions(&whole).await?;

// Prepare `system.remark` call.
let remark_call = tester.api.call().system().remark(vec![])?;
// Prepare `settlement.create_venue` call.
Expand Down Expand Up @@ -317,3 +325,51 @@ async fn multisig_as_secondary_key_change_identity() -> Result<()> {

Ok(())
}

#[tokio::test]
async fn secondary_key_creates_multisig() -> Result<()> {
let mut tester = PolymeshTester::new().await?;
// Create a user with secondary keys to be the MS creator.
let users = tester
.users_with_secondary_keys(&[("MS_Creator_DID_with_sk", 1)])
.await?;
let did1 = users[0].clone();
let sk = did1.get_sk(0)?.clone();

// Create a MS but don't link it to an identity.
let mut ms = MuliSigState::create(&mut tester, &sk, 3, 2).await?;

// Prepare `system.remark` call.
let remark_call = tester.api.call().system().remark(vec![])?;
// Prepare `settlement.create_venue` call.
let create_venue_call = tester.api.call().settlement().create_venue(
VenueDetails(vec![]),
vec![],
VenueType::Other,
)?;

let expected = vec![
true, // remark.
false, // create venue. MS has no DID, so this fails.
];
let batch_call = tester.api.call().utility().force_batch(vec![
remark_call.runtime_call().clone(),
create_venue_call.runtime_call().clone(),
])?;

let mut res = ms.run_proposal(batch_call).await?;
res.ok().await?;
let calls_ok = get_batch_results(&mut res).await?;
assert_eq!(calls_ok, expected);

// A secondary key can't make the MS a primary.
let res = ms.make_primary().await?.ok().await;
assert!(res.is_err());

let whole = PermissionsBuilder::whole();
// A secondary key can't make the MS a secondary key with custom permissions.
let res = ms.make_secondary(Some(whole.into())).await?.ok().await;
assert!(res.is_err());

Ok(())
}
Loading

0 comments on commit fa4ccd8

Please sign in to comment.