Repository navigation
feat: add coloridentifier module for colour id derivation - #29
Conversation
azuchi
left a comment
There was a problem hiding this comment.
新モジュール自体は正しく、マージを妨げる問題は見つかりませんでした。手元で確認した内容です。
./scripts/check.shは 3,138 件パス、format:ciとlint:testsも通り、ビルド後のsrc//types/に差分なしMetadata.deriveColorIdは、ベースブランチの実装とランダムな鍵・outpoint 200 件(3 種のトークンタイプすべて)で完全一致- tapyrus-core の
COutPointの直列化(hashMalFix‖n)、ColorIdentifier(const CScript&)の導出、coloridentifier_tests.cppの 2 ベクタと整合
ただし、この PR が明文化した OutPoint の契約と既存の Metadata.fetch が食い違っていることを、レジストリの実データで確認しました。この PR 以前からある不具合なのでブロッキングとはしませんが、同じ PR か直後の PR での対応をおすすめします。詳細はインラインに書きました。それ以外は非ブロッキングの提案です。
| * transaction in the byte order it is serialized in, which is the reverse of | ||
| * the order it is displayed in. | ||
| */ | ||
| export interface OutPoint { |
There was a problem hiding this comment.
対応をおすすめします。 ここで txid を「直列化順(表示順の逆)」と定義していますが、Metadata.fetch(ts_src/metadata.ts:137-141)はレジストリ JSON の hex をそのまま Buffer.from(data.outpoint.txid, 'hex') にしており、この契約を満たしていません。
tapyrus-token-registry の docs/tokens/ で outpoint を持つ実エントリ 2 件(c21c6955f3… と c3e26287c1…)について、type ‖ SHA256(txid ‖ index) をファイル名の colorId と照合しました。
| txid の扱い | colorId と一致 |
|---|---|
| JSON の hex をそのまま使う | 0 / 2 |
| 反転して使う | 2 / 2 |
レジストリは txid を表示順で保存しているので、fetch() が返した entry.outPoint をそのまま deriveColorId や coloridentifier.nft / nonReissuable に渡すと、例外なしに tapyrus-core と一致しない colorId が返ります。
この PR 以前からある不具合ですが(旧コードも txid をそのままコピーしていました)、契約を明文化した今が直しどきだと思います。fetch の中で reverseBuffer し、実エントリを使って「fetch の戻り値から導出した colorId がリクエストした colorId と一致する」ことを確認するテストを足すのがよさそうです。
There was a problem hiding this comment.
Metadata.fetch でレジストリの txid を reverseBuffer するようにしました。
| * from a script containing OP_COLOR. | ||
| */ | ||
| export function reissuable(scriptPubKey: Buffer): Buffer { | ||
| typeforce(typeforce.Buffer, scriptPubKey); |
There was a problem hiding this comment.
非ブロッキングです。doc コメントは「scriptPubKey は色を含んではならない」としていますが、検証は Buffer かどうかだけなので、CP2PKH のような彩色スクリプトや空の Buffer を渡しても、0xc1 で始まる有効に見える colorId が返ります。
coloridentifier.reissuable(payments.cp2pkh({ hash, colorId }).output) // -> c1fb4d9e… 例外なし
coloridentifier.reissuable(Buffer.alloc(0)) // -> c1e3b0c4… 例外なしtapyrus-core の RPC(src/wallet/rpcwallet.cpp)は IsColoredScript() なら Script input for tokens cannot be another token script. で明示的に拒否しています。ここでも彩色スクリプトは弾くのがよいと思います。test/coloridentifier.spec.ts:91 は空 Buffer で 33 バイトが返ることを正として固定しているので、あわせて見直しが必要です。
なお記述の正確さとして、tapyrus-core の ColorIdentifier(const CScript&) コンストラクタ自体は無条件に導出します。色付きスクリプトからの発行を認めないのは RPC と検証(validation.cpp は TPC 入力のときだけこのコンストラクタを使う)の挙動なので、「derives no colour from a script containing OP_COLOR」はそのように書くのが正確です。
There was a problem hiding this comment.
reissuable は OP_COLOR を含むスクリプトを TypeError で拒否するようにしました。
判定は tapyrus-core の CScript::IsColoredScript に合わせ、script.decompile した結果に OP_COLOR があるかを見ます。先頭形式(CP2PKH / CP2SH)だけでなく、位置を問わず拒否します。
doc コメントは、指摘のとおり RPC と検証の挙動として書き直しました。
空の Buffer は拒否していません。tapyrus-core の IsColoredScript も空スクリプトでは false を返すため、これに合わせました。test/coloridentifier.spec.ts の「33 バイトを返す」テストは、空 Buffer をやめて tapyrus-core のベクタのスクリプトに変えています。
| // tapyrusrb spec/tapyrus/tip0137_spec.rb | ||
| assert.strictEqual( | ||
| coloridentifier | ||
| .nft({ txid: Buffer.alloc(32, 0x01), index: 1 }) |
There was a problem hiding this comment.
非ブロッキングです。2 点あります。
-
このベクタの txid は 0x01 の 32 バイト埋めで、反転しても同じバイト列になります。将来
serializeOutPointが誤って txid を反転するようになっても、このテストは通ります。バイト順を固定しているのはnonReissuableの tapyrus-core ベクタ 1 件だけで、test/metadata.spec.ts側の outPoint もBuffer.alloc(32, 0x01 / 0x02)なので、Metadata 経路のバイト順は固定されていません。回文でない txid のベクタを nft と Metadata 経路にも 1 件ずつ足しておくと安心です。 -
出典のコメントは
tip0137_spec.rbになっていますが、そちらはこの値を定数として使っているだけで、導出を検証しているのは tapyrusrb のspec/tapyrus/script/color_spec.rb(ColorIdentifier.nft(OutPoint.new("01" * 32, 1)))です。値と導出元の一致は確認しました。
There was a problem hiding this comment.
- tapyrus-core の
non_reissuableベクタ(485273f6…、回文ではありません)を使い、nftとMetadata.deriveColorId(non_reissuableとnft)にバイト順のテストを足しました。nftは、non_reissuableの値と型バイトだけが違うことで固定しています。 - 出典コメントを
tip0137_spec.rbからspec/tapyrus/script/color_spec.rbに直しました。
|
|
||
| function serializeOutPoint(outPoint: OutPoint): Buffer { | ||
| typeforce( | ||
| { txid: typeforce.BufferN(TXID_LENGTH), index: typeforce.UInt32 }, |
There was a problem hiding this comment.
細かい点です。./types に既に Hash256bit(= BufferN(32))と UInt32 があるので、それを使えば typeforce の直接 require と TXID_LENGTH を使ったスキーマの組み立てが不要になります。スキーマも呼び出しごとに組み立て直しているので、モジュールレベルで 1 度だけ typeforce.compile しておく形にもできます。
There was a problem hiding this comment.
Hash256bit と UInt32 を使い、schema をモジュールレベルで 1 度だけ typeforce.compile する形にしました。typeforce の require は compile のために残しています。エラーメッセージは変わりません。
| @@ -1,3 +1,5 @@ | |||
| import * as coloridentifier from './coloridentifier'; | |||
| import { OutPoint } from './coloridentifier'; | |||
There was a problem hiding this comment.
細かい点です。同じ ./coloridentifier を名前空間 import と名前付き import で 2 回読み込み、30 行目で型を値の構文の export { OutPoint } で再エクスポートしています。tsc と lint は通っていますが、export type { OutPoint } from './coloridentifier';(または export { OutPoint } from './coloridentifier';)の 1 行にまとめ、deriveColorId の引数型は coloridentifier.OutPoint で参照すると、import が 1 つで済みます。
There was a problem hiding this comment.
export { OutPoint } from './coloridentifier'; の 1 行にまとめました。RegistryEntry と deriveColorId の型は coloridentifier.OutPoint で参照します
6ce7f2a to
a87b2a4
Compare
|
@Yamaguchi please fix conflicts. |
a87b2a4 to
591b689
Compare
Summary
Deriving a colour identifier was possible only through
Metadata.deriveColorId,which needs a full TIP-0020 metadata document. The two are separate concepts:
tapyrus-core keeps the derivation in
src/coloridentifier.h, independent of anymetadata, and so does tapyrusrb (
ColorIdentifier.reissuable/.non_reissuable/.nft). Anyone issuing a token without metadata had toreimplement it.
ts_src/coloridentifier.tsexposes the three derivations directly:reissuable(scriptPubKey)0xc1then SHA256 of the scriptnonReissuable(outPoint)0xc2then SHA256 oftxid ‖ indexnft(outPoint)0xc3then SHA256 oftxid ‖ indexOutPoint.txidholds the hashMalFix in serialization order, the reverse of theorder it is displayed in, so it matches what
Transaction.getMalFixHash()returns.
Metadata keeps its behaviour
Metadata.deriveColorIdnow calls this module. Its arguments and its returnvalue are unchanged. What goes away is the duplication: the outpoint was packed
by hand twice, and a P2PKH script was assembled byte by byte in a third place.
OutPointmoved tocoloridentifier.tsandmetadata.tsre-exports it, soimport { OutPoint } from 'tapyrusjs-lib'and themetadataentry point bothstill resolve.
Validation the old code did not do
nonReissuableandnftnow check their argument. A txid that is not 32 bytesand an index outside uint32 are refused by typeforce with a message that names
the field. Before, a 31 byte txid produced a colour identifier over a
zero-padded buffer and a non-integer index produced one silently, while a
negative index threw Node's
ERR_OUT_OF_RANGEfromwriteUInt32LE. Validinput gives the same answer as before; only the bad input changed, from a wrong
identifier to a
TypeError.Affected areas
ts_src/coloridentifier.ts(new),ts_src/metadata.ts,ts_src/index.ts.coloridentifieris exported from the package root.How to verify
3,138 tests pass.
test/coloridentifier.spec.tsis new and pins the two vectorstapyrus-core asserts in
src/test/coloridentifier_tests.cpp(
coloridentifier_string_conversion):Both were reproduced by hand before the module was written, which is also how
the byte order of the outpoint was settled: reversing the txid gives a different
identifier, and the reversed one does not match tapyrus-core.
Two fixed vectors pin the derivation
Metadata.deriveColorIdandcoloridentifier.nftproduce, so a future change topayments.p2pkhor to theoutpoint packing can not pass unnoticed. Both were checked to give the same
answer on the previous code.
Beyond that this is a refactor, so there is no test that fails against it. The
old derivation was written out separately and compared against the new one over
200 random keys; all 200 agree.
Base branch
Stacked on
fix/validate_color_id_type_byte, whereCOLOR_ID_REISSUABLEandits two siblings are defined. Please merge that first; GitHub retargets this PR
at
masterwhen it does.