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

Fix blank transition builder #108

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 45 additions & 4 deletions std/src/interface/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,17 @@

use std::collections::{BTreeSet, HashSet};

use amplify::confinement::{LargeOrdMap, LargeVec, SmallVec};
use amplify::confinement::{LargeOrdMap, LargeVec, SmallVec, U16};
use bp::Outpoint;
use rgb::{
AssignmentType, AttachId, ContractId, ContractState, FungibleOutput, MediaType, RevealedAttach,
RevealedData, SealWitness,
AssignmentType, AttachId, ContractId, ContractState, DataOutput, FungibleOutput, MediaType,
RevealedAttach, RevealedData, SealWitness,
};
use strict_encoding::FieldName;
use strict_encoding::{FieldName, StrictDeserialize};
use strict_types::typify::TypedVal;
use strict_types::{decode, StrictVal};

use crate::interface::rgb21::Allocation;
use crate::interface::IfaceImpl;

#[derive(Clone, Eq, PartialEq, Debug, Display, Error, From)]
Expand Down Expand Up @@ -94,6 +95,28 @@
}
}

#[derive(Clone, Eq, PartialEq, Debug)]

Check warning on line 98 in std/src/interface/contract.rs

View check run for this annotation

Codecov / codecov/patch

std/src/interface/contract.rs#L98

Added line #L98 was not covered by tests
pub struct DataAllocation {
pub owner: Outpoint,
pub witness: SealWitness,
pub value: Allocation,
}

impl From<DataOutput> for DataAllocation {
fn from(out: DataOutput) -> Self { Self::from(&out) }

Check warning on line 106 in std/src/interface/contract.rs

View check run for this annotation

Codecov / codecov/patch

std/src/interface/contract.rs#L106

Added line #L106 was not covered by tests
}

impl From<&DataOutput> for DataAllocation {
fn from(out: &DataOutput) -> Self {
DataAllocation {
owner: out.seal,
witness: out.witness,
value: Allocation::from_strict_serialized::<U16>(out.state.as_ref().to_owned())
.expect("invalid allocation data"),
}
}

Check warning on line 117 in std/src/interface/contract.rs

View check run for this annotation

Codecov / codecov/patch

std/src/interface/contract.rs#L110-L117

Added lines #L110 - L117 were not covered by tests
}

pub trait OutpointFilter {
fn include_outpoint(&self, outpoint: Outpoint) -> bool;
}
Expand Down Expand Up @@ -182,6 +205,24 @@
Ok(LargeVec::try_from_iter(state).expect("same or smaller collection size"))
}

pub fn data(
&self,
name: impl Into<FieldName>,
) -> Result<LargeVec<DataAllocation>, ContractError> {
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to add outpoint filter like in fungible method above

let name = name.into();
let type_id = self
.iface
.assignments_type(&name)
.ok_or(ContractError::FieldNameUnknown(name))?;
let state = self
.state
.data()
.iter()
.filter(|outp| outp.opout.ty == type_id)
.map(DataAllocation::from);
Ok(LargeVec::try_from_iter(state).expect("same or smaller collection size"))
}

Check warning on line 224 in std/src/interface/contract.rs

View check run for this annotation

Codecov / codecov/patch

std/src/interface/contract.rs#L208-L224

Added lines #L208 - L224 were not covered by tests

// TODO: Add rights, attachments and structured data APIs
pub fn outpoint(
&self,
Expand Down
3 changes: 3 additions & 0 deletions std/src/interface/rgb21.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@
pub fn with(index: TokenIndex, fraction: OwnedFraction) -> Allocation {
Allocation(index, fraction)
}

pub fn token_id(self) -> TokenIndex { self.0 }
Copy link
Member

Choose a reason for hiding this comment

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

Id's are usually hashes, thus better to use the type name token_index

pub fn fraction(self) -> OwnedFraction { self.1 }

Check warning on line 106 in std/src/interface/rgb21.rs

View check run for this annotation

Codecov / codecov/patch

std/src/interface/rgb21.rs#L105-L106

Added lines #L105 - L106 were not covered by tests
}

impl StrictSerialize for Allocation {}
Expand Down
10 changes: 9 additions & 1 deletion std/src/persistence/hoard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@

use crate::accessors::{MergeReveal, MergeRevealError};
use crate::containers::{Cert, Consignment, ContentId, ContentSigs};
use crate::interface::{rgb20, ContractSuppl, Iface, IfaceId, IfacePair, SchemaIfaces};
use crate::interface::{
rgb20, rgb21, rgb25, ContractSuppl, Iface, IfaceId, IfacePair, SchemaIfaces,
};
use crate::persistence::{InventoryError, Stash, StashError, StashInconsistency};
use crate::LIB_NAME_RGB_STD;

Expand Down Expand Up @@ -78,10 +80,16 @@
pub fn preset() -> Self {
let rgb20 = rgb20();
let rgb20_id = rgb20.iface_id();
let rgb21 = rgb21();
let rgb21_id = rgb21.iface_id();
let rgb25 = rgb25();
let rgb25_id = rgb25.iface_id();

Check warning on line 86 in std/src/persistence/hoard.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/hoard.rs#L83-L86

Added lines #L83 - L86 were not covered by tests
Hoard {
schemata: none!(),
ifaces: tiny_bmap! {
rgb20_id => rgb20,
rgb21_id => rgb21,
rgb25_id => rgb25,

Check warning on line 92 in std/src/persistence/hoard.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/hoard.rs#L91-L92

Added lines #L91 - L92 were not covered by tests
},
geneses: none!(),
suppl: none!(),
Expand Down
28 changes: 22 additions & 6 deletions std/src/persistence/inventory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,29 @@
let schema_ifaces = self.contract_schema(contract_id)?;
let iface = self.iface_by_name(&iface.into())?;
let schema = &schema_ifaces.schema;
let iimpl = schema_ifaces
.iimpls
.get(&iface.iface_id())
.ok_or(DataError::NoIfaceImpl(schema.schema_id(), iface.iface_id()))?;
let builder =

if schema_ifaces.iimpls.is_empty() {
return Err(InventoryError::DataError(DataError::NoIfaceImpl(
schema.schema_id(),
iface.iface_id(),
)));
}

Check warning on line 419 in std/src/persistence/inventory.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/inventory.rs#L413-L419

Added lines #L413 - L419 were not covered by tests

let builder = if let Some(iimpl) = schema_ifaces.iimpls.get(&iface.iface_id()) {

Check warning on line 421 in std/src/persistence/inventory.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/inventory.rs#L421

Added line #L421 was not covered by tests
TransitionBuilder::blank_transition(iface.clone(), schema.clone(), iimpl.clone())
.expect("internal inconsistency");
.expect("internal inconsistency")

Check warning on line 423 in std/src/persistence/inventory.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/inventory.rs#L423

Added line #L423 was not covered by tests
} else {
let (default_iface_id, default_iimpl) = schema_ifaces.iimpls.first_key_value().unwrap();
let default_iface = self.iface_by_id(*default_iface_id)?;

Check warning on line 426 in std/src/persistence/inventory.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/inventory.rs#L425-L426

Added lines #L425 - L426 were not covered by tests

TransitionBuilder::blank_transition(
default_iface.clone(),
schema.clone(),
default_iimpl.clone(),
)
.expect("internal inconsistency")

Check warning on line 433 in std/src/persistence/inventory.rs

View check run for this annotation

Codecov / codecov/patch

std/src/persistence/inventory.rs#L428-L433

Added lines #L428 - L433 were not covered by tests
};

Ok(builder)
}

Expand Down
12 changes: 10 additions & 2 deletions std/src/stl/specs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,8 +477,9 @@
pub fn details(&self) -> Option<&str> { self.naming.details.as_ref().map(|d| d.as_str()) }
}

#[derive(Clone, Eq, PartialEq, Debug, Default)]
#[derive(StrictType, StrictEncode, StrictDecode)]
#[derive(Wrapper, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, From, Debug)]

Check warning on line 480 in std/src/stl/specs.rs

View check run for this annotation

Codecov / codecov/patch

std/src/stl/specs.rs#L480

Added line #L480 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Why Default implementation is removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Default causes conflict with strict_dumb. Do you prefer maintain Default?

Copy link
Member

Choose a reason for hiding this comment

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

I think having default is better

#[wrapper(Deref, Display)]
Copy link
Member

Choose a reason for hiding this comment

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

Since we are adding Wrapper we need to add FromStr here and remove manual FromStr implementation below

#[derive(StrictDumb, StrictType, StrictEncode, StrictDecode)]
#[strict_type(lib = LIB_NAME_RGB_CONTRACT)]
#[cfg_attr(
feature = "serde",
Expand All @@ -498,6 +499,13 @@
}
}

impl RicardianContract {
pub fn from_strict_val_unchecked(value: &StrictVal) -> Self {
let s = value.unwrap_string();
RicardianContract::from_str(&s).expect("invalid term data")
Copy link
Member

Choose a reason for hiding this comment

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

This will open a vulnerability where an attacker can send a malformed consignment and crash end-user wallets and nodes (DoS attack).

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't know, as I saw in several code snippets, I imagined that this was the correct implementation. What would be the alternative?

Copy link
Member

Choose a reason for hiding this comment

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

Why do you need an unchecked version?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a method that extracts some information from the ContractIface::global method, and this method returns the StrictVal array. I use from_strict_val_unchecked to convert StrictVal to a specific structure.

Copy link
Member

Choose a reason for hiding this comment

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

Can you point specific place in code where you do that call?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Why you do not want to use try_from and just propagate the error?

}

Check warning on line 506 in std/src/stl/specs.rs

View check run for this annotation

Codecov / codecov/patch

std/src/stl/specs.rs#L503-L506

Added lines #L503 - L506 were not covered by tests
}

#[derive(Wrapper, WrapperMut, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Debug, From)]
#[wrapper(Deref, Display, FromStr, MathOps)]
#[wrapper_mut(DerefMut, MathAssign)]
Expand Down
Loading