RGB-WG / rgb-core

RGB Core Library: consensus validation for private & scalable client-validated smart contracts on Bitcoin & Lightning
https://spec.rgb.tech
Apache License 2.0
207 stars 52 forks source link

Audit and refactoring of commitments workflows. Vesper support #204

Closed dr-orlovsky closed 8 months ago

codecov[bot] commented 9 months ago

Codecov Report

Attention: Patch coverage is 9.44056% with 259 lines in your changes are missing coverage. Please review.

Project coverage is 16.0%. Comparing base (52e9dcb) to head (1759eab). Report is 2 commits behind head on master.

:exclamation: Current head 1759eab differs from pull request most recent head d4f96c9. Consider uploading reports for the commit d4f96c9 to get more accurate results

Files Patch % Lines
src/contract/id.rs 12.0% 117 Missing :warning:
src/bin/rgbcore-stl.rs 0.0% 50 Missing :warning:
src/lib.rs 2.6% 37 Missing :warning:
src/contract/operations.rs 16.7% 10 Missing :warning:
src/contract/state.rs 0.0% 10 Missing :warning:
src/validation/state.rs 0.0% 9 Missing :warning:
src/contract/bundle.rs 27.3% 8 Missing :warning:
src/contract/data.rs 0.0% 7 Missing :warning:
src/contract/attachment.rs 0.0% 4 Missing :warning:
src/schema/schema.rs 0.0% 3 Missing :warning:
... and 2 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #204 +/- ## ======================================== - Coverage 16.8% 16.0% -0.7% ======================================== Files 32 33 +1 Lines 3896 4001 +105 ======================================== - Hits 653 642 -11 - Misses 3243 3359 +116 ``` | [Flag](https://app.codecov.io/gh/RGB-WG/rgb-core/pull/204/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RGB-WG) | Coverage Δ | | |---|---|---| | [rust](https://app.codecov.io/gh/RGB-WG/rgb-core/pull/204/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RGB-WG) | `16.0% <9.4%> (-0.7%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=RGB-WG#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

crisdut commented 9 months ago

Hi @dr-orlovsky,

I noticed that this change causes an error in the contracts of RGB20 and RGB25: Every time we execute the contract_id method, the method returns a different id.

Because this doesn't happen with RGB21, I assume it's related to AssetTags.

!!! Update !!!

After some verification, I noticed this change causes the error: https://github.com/RGB-WG/rgb-core/pull/206/files#diff-bc03f4e1f24cc17c9393d8df61c0d41c5801bd6c8b8b8b58f80a37fbf738b028R467-R468

dr-orlovsky commented 9 months ago

@crisdut yeah, I see the reason. Will work on fix

dr-orlovsky commented 9 months ago

@crisdut thank you for finding the source of the issue. In https://github.com/RGB-WG/rgb-core/pull/204/commits/86fb38229b43c2ffcf274f0f0e1c7b957a10bf20 I fixed the code such that it is impossible to call a method returning an invalid commitment value.

crisdut commented 9 months ago

@dr-orlovsky

I tried transfer with all latest version of the crates and I find a new error:

commit encoder can commit only to named types

After investigate, we try commit a unnamed type here (the input_map):

pub struct TransitionBundle {
    pub input_map: Confined<BTreeMap<Vin, OpId>, 1, U16>,
    pub known_transitions: Confined<BTreeMap<OpId, Transition>, 1, U16>,
}

impl CommitEncode for TransitionBundle {
    type CommitmentId = BundleId;

    fn commit_encode(&self, e: &mut CommitEngine) { e.commit_to(&self.input_map); } <------ error here
}

The error occurs because Confined::<BTreeMap<Vin, OpId>, 1, U16>::strict_name() is implemented by:

// ./strict_enconding/rust/src/embedded.rs:547

impl<K: StrictType + Ord + Hash, V: StrictType, const MIN_LEN: usize, const MAX_LEN: usize>
    StrictType for Confined<BTreeMap<K, V>, MIN_LEN, MAX_LEN>
{
    const STRICT_LIB_NAME: &'static str = LIB_EMBEDDED;
    fn strict_name() -> Option<TypeName> { None }
}

While the other types evoke another impl:

// ./strict_enconding/rust/src/types.rs:48
pub trait StrictType: Sized {
    const STRICT_LIB_NAME: &'static str;
    fn strict_name() -> Option<TypeName> {
        fn get_ident(path: &str) -> &str {
            path.rsplit_once("::")
                .map(|(_, n)| n.trim())
                .unwrap_or(path)
        }

        let name = any::type_name::<Self>().replace('&', "");
        let mut ident = vec![];
        for mut arg in name.split([',', '<', '>', '(', ')']) {
            arg = arg.trim();
            if arg.is_empty() {
                continue;
            }
            ident.push(get_ident(arg));
        }
        Some(tn!(ident.join("")))
    }
}
dr-orlovsky commented 9 months ago

@crisdut thanks for reporting the details of the source of the error. Fixed in https://github.com/RGB-WG/rgb-core/pull/204/commits/1759eab75a1d2f94f402db6897bc76abc1221441