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 1 commit
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 @@ impl From<&FungibleOutput> for FungibleAllocation {
}
}

#[derive(Clone, Eq, PartialEq, Debug)]
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) }
}

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"),
}
}
}

pub trait OutpointFilter {
fn include_outpoint(&self, outpoint: Outpoint) -> bool;
}
Expand Down Expand Up @@ -182,6 +205,24 @@ impl ContractIface {
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"))
}

// 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 @@ impl Allocation {
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 }
}

impl StrictSerialize for Allocation {}
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 @@ impl DivisibleAssetSpec {
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)]
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 FromStr for RicardianContract {
}
}

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?

}
}

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