Page MenuHomePhabricator

[Chronik] Add `slp::parse`, with support for SLP GENESIS txs
ClosedPublic

Authored by tobias_ruck on Dec 20 2023, 00:22.

Details

Summary

Parses SLP txs, and for now we only add support for parsing GENESIS txs.

GENESIS txs create a new token with its own ID, which is the txid of the GENESIS tx.

Depends on D15009 and D15011.

Test Plan

cargo test -p bitcoinsuite-slp

Diff Detail

Repository
rABC Bitcoin ABC
Branch
chronik-slp-genesis
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 26006
Build 51586: Build Diffbuild-chronik · chronik-client-integration-tests
Build 51585: arc lint + arc unit

Event Timeline

tobias_ruck updated this revision to Diff 43682.

added copyright notices

add test for invalid 2 byte token type

chronik/bitcoinsuite-slp/src/slp/error.rs
132

Didn't we commit to use a single byte for the token type ?

143

I'm not sure about this method. It's weird to have the interpretation of the error be an method of the error itself

osoftware added inline comments.
chronik/bitcoinsuite-slp/src/slp/error.rs
143

And wherever this should go, a more meaningful name would be: unknown_protocol or something like that.

Or maybe consider moving this logic a layer up where we would have some fn converting Result<ParsedData, ParseError> -> SomeEnum where SomeEnum would be something like ValidToken(ParsedData) | InvalidToken(ParseError) | UnknownProtocol. I don't have a clear picture yet.

chronik/bitcoinsuite-slp/src/slp/error.rs
132

good catch!

143

@Fabien and I discussed this on TG, and we're going with Result<Option<ParsedData>, ParseError>.

I also added the same change to the eMPP code, see D15016, so this will look similar here.

change signature of parse and parse_tx to Result<Option<ParsedData>, ParseError>.
this parallels D15016.

added MintField to simplify the mint_baton_out_idx vs. mint_vault_scripthash handling code

Fabien requested changes to this revision.Dec 20 2023, 20:33
Fabien added inline comments.
chronik/bitcoinsuite-slp/src/slp/error.rs
117 ↗(On Diff #43698)

This could actually be a slightly different error and simplify the code a bit: UnexpectedPushNumber with expected and actual that throws when expected != actual (and not only when actual < expected). Doing so removes the need for another check for SuperfluousPushes.

chronik/bitcoinsuite-slp/src/slp/genesis.rs
35 ↗(On Diff #43698)

Here: little value to check < and > as 2 different conditions since you know the exact number of push you expect, could be just:

if opreturn_data.len() != 10 {
    return Err(ParseError::UnexpectedPushNumber {
        expected: 10,
	actual: opreturn_data.len(),
    });
}
63 ↗(On Diff #43698)

We discussed the opportunity to make this an enum instead, which looks like a good improvement

chronik/bitcoinsuite-slp/src/slp/parse.rs
82 ↗(On Diff #43698)

great

149 ↗(On Diff #43698)

Nit: add this is reflected in the spec from the bcash repo

This revision now requires changes to proceed.Dec 20 2023, 20:33

simplify using UnexpectedNumPushes, add test case for token_document_hash

Fabien added inline comments.
chronik/bitcoinsuite-slp/tests/test_slp_parse_genesis.rs
338 ↗(On Diff #43702)

good catch

This revision is now accepted and ready to land.Dec 20 2023, 21:34