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(rust/cardano-blockchain-types): fix CIP36 #133

Merged
merged 21 commits into from
Jan 6, 2025

Conversation

bkioshn
Copy link
Contributor

@bkioshn bkioshn commented Jan 2, 2025

Description

  • Improve CIP36 error handling
    • Error besides from decoding the CBOR should be accumulate into a record and return all errors.
  • Remove decode_helper.rs and use cbor-utils
  • Move cip36 folder to be under metadata
  • Modify Cip36 constructor (new) to accept MultiEraBlock, txn_idx, and is_catalyst_strict, all the logic needed to construct Cip36 is implemented here instead of in cardano-chain-follower
  • Cip36 struct now contains validation field.

Related Pull Requests

As discuss in #125
and #123

Test

Test with cardano-chain-follower

  1. Valid data:
    https://preprod.cardanoscan.io/transaction/5c7cf7bff543447fc1e46c2598380fa08b8d882de2191ef5bc80078b71724217?tab=metadata
follow_chains: CIP36 TxnIndex(1): Cip36 { key_registration: Cip36KeyRegistration { is_cip36: Some(false), voting_pks: [VotingPubKey { voting_pk: Some(VerifyingKey(CompressedEdwardsY: [221, 169, 229, 65, 141, 15, 186, 201, 214, 36, 163, 166, 226, 89, 203, 193, 148, 245, 226, 238, 245, 139, 145, 194, 194, 198, 117, 251, 176, 154, 77, 94]), EdwardsPoint{
        X: FieldElement51([1859631604253382, 2110668188143661, 2170145193671975, 1856524863429020, 518017364888712]),
        Y: FieldElement51([580049323796957, 1219171104512311, 998177562373479, 951495390788343, 1658998412523356]),
        Z: FieldElement51([1, 0, 0, 0, 0]),
        T: FieldElement51([761601465539487, 1727344332581539, 842271326284158, 364342582718309, 1346852481195048])
})), weight: 1 }], stake_pk: Some(VerifyingKey(CompressedEdwardsY: [18, 165, 183, 33, 41, 3, 240, 23, 34, 50, 61, 34, 154, 8, 173, 145, 232, 176, 22, 89, 170, 211, 40, 76, 10, 173, 85, 108, 255, 153, 47, 147]), EdwardsPoint{
        X: FieldElement51([823749255044963, 1222778930982365, 998188060869874, 536412584153685, 1545542024390386]),
        Y: FieldElement51([3475194225938, 919499452990206, 1225696194507810, 1835248457995564, 337522688705882]),
        Z: FieldElement51([1, 0, 0, 0, 0]),
        T: FieldElement51([355660423901968, 2073445582995877, 1935667460124604, 2192713943034174, 995866259383485])
})), payment_addr: Some(ShelleyAddress(Testnet, Key(Hash<28>("afe07feb59dda08fdbac532c0d65e9c302366ebfeac91fa64ac8dabf")), Key(Hash<28>("464fbac58c3cac691bbd379e112318dc41854a5283b34e07905beece")))), nonce: Some(55076993), purpose: 0, raw_nonce: Some(55076993), is_payable: Some(true) }, registration_witness: Cip36RegistrationWitness { signature: Some(ed25519::Signature { R: 0x5fc485d5204599c402becdaf0552f1e7d9c499525939795bddc13355abdbd676, s: 0xcf06b66d2b82cfbe0bbf55aad2cc72bd5f47b72f89bfed513dab9d638c35860b }) }, is_catalyst_strict: true }, network: "Preprod", block: 2050484
  1. Invalid data: The metadata contain 3: e0ce4.... which e0 = Testnet StakeKeyHash
    https://preprod.cardanoscan.io/transaction/3c9f692981e37502f17268e3d54e5a4ab19aa1b110643360f030eb88655d0f03?tab=metadata
follow_chains: Failed to decode CIP36 TxnIndex(2): Failed to construct CIP-36 key registration, decode error: ["Invalid CIP36 Key Registration payment address, expected Shelley address, got stake_test1ur8y7xypuu6n5pv6xl6v8rlwfktwu9wk3q8l3avqk5h7exq44mt99"], network: "Preprod", block: 281062
  1. Invalid data, multiple errors
    https://preprod.cardanoscan.io/transaction/605099f26e6ce48e1678e54ccfda223745a53423d4ba954f34e768427e85f010?tab=metadata
follow_chains: Failed to decode CIP36 TxnIndex(0): Failed to construct CIP-36 key registration, decode error: ["CIP36 Key Registration voting key, multiple voting keys, Failed to convert to VerifyingKey: signature::Error { source: Some(Cannot decompress Edwards point) }", "CIP36 Key Registration voting key, multiple voting keys, Failed to convert to VerifyingKey: signature::Error { source: Some(Cannot decompress Edwards point) }", "Invalid CIP36 Key Registration payment address, expected Shelley address, got stake_test1uruhytm36gm9gwr7cyuflcjn6wqx20600eest2qv7hzdlggeua0e4"], network: "Preprod", block: 206866

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@bkioshn bkioshn marked this pull request as draft January 2, 2025 12:04
@bkioshn bkioshn self-assigned this Jan 2, 2025
Copy link
Contributor

github-actions bot commented Jan 2, 2025

Test Report | ${\color{lightgreen}Pass: 264/264}$ | ${\color{red}Fail: 0/264}$ |

Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

Can we move cip36 into a metadata/cip36 module instead of at the root of the types. This is because we are likely to add more decoded metadata handlers and we should structure that more logically. It does not belong under auxdata because thats raw aux data whereas this is decoded metadata which uses it.

Copy link
Contributor

github-actions bot commented Jan 4, 2025

Test Report | ${\color{lightgreen}Pass: 279/279}$ | ${\color{red}Fail: 0/279}$ |

Copy link
Contributor

github-actions bot commented Jan 4, 2025

Test Report | ${\color{lightgreen}Pass: 279/279}$ | ${\color{red}Fail: 0/279}$ |

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Test Report | ${\color{lightgreen}Pass: 279/279}$ | ${\color{red}Fail: 0/279}$ |

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Test Report | ${\color{lightgreen}Pass: 279/279}$ | ${\color{red}Fail: 0/279}$ |

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Test Report | ${\color{lightgreen}Pass: 279/279}$ | ${\color{red}Fail: 0/279}$ |

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Test Report | ${\color{lightgreen}Pass: 279/279}$ | ${\color{red}Fail: 0/279}$ |

@bkioshn bkioshn marked this pull request as ready for review January 6, 2025 03:06
@bkioshn bkioshn requested a review from stevenj January 6, 2025 03:06
@bkioshn bkioshn merged commit 543b893 into fix/cardano-bc-types-base-change Jan 6, 2025
22 checks passed
@bkioshn bkioshn deleted the fix/cip36 branch January 6, 2025 04:28
@bkioshn bkioshn restored the fix/cip36 branch January 6, 2025 07:33
apskhem pushed a commit that referenced this pull request Jan 9, 2025
…hange (#123)

* feat(rust): add cardano-blockchain-types crate

* fix(rust): Remove unused dependencies

* fix(cardano-blockchain-types): time_to_slot calculation

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): remove justfile

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): point new should take type Slot and Blake2bHash

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): Fork type

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): point and fuzzy point test

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add Fork increment function

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add comment on tag 259

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add Fork decrement function

Signed-off-by: bkioshn <[email protected]>

* test(rust): try earthly no-cache

Signed-off-by: bkioshn <[email protected]>

* test(rust): try earthly no-cache and fix doc artifact

Signed-off-by: bkioshn <[email protected]>

* test(rust): remove no-cache

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): expose Fork and Network

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add partailOrd to Fork

Signed-off-by: bkioshn <[email protected]>

* Update rust/cardano-blockchain-types/src/point.rs

Co-authored-by: Stanislav Tkach <[email protected]>

* fix(cardano-blockchain-types): cleanup

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): testdoc

Signed-off-by: bkioshn <[email protected]>

* Update rust/cardano-blockchain-types/src/point.rs

* fix(cardano-blockchain-types): cleanup

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

* fix(cardano-blockchain-types): add validate PR title

* fix(cardano-blockchain-types): comments

* fix(cardano-blockchain-types): fix hash_or_default

* fix(cardano-blockchain-types): redundant code

* test: no cache

* test: revert change

* test ci

* test ci

* test ci

* test ci

* test ci

* test ci

* test ci

* test ci

* test ci

* test ci

* revert change

* test ci

* revert change

* fix(rust/cardano-blockchain-types): add more functionality to `Slot` (#124)

* fix(cardano-blockchain-types): add more trait to slot

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add serde serialize to slot

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): use sat_mul for slot

Signed-off-by: bkioshn <[email protected]>

---------

Signed-off-by: bkioshn <[email protected]>

* feat(rust/cardano-blockchain-types): Add CIP36 (#125)

* feat(cardano-blockchain-types): add cip36

Signed-off-by: bkioshn <[email protected]>

* feat(cardano-blockchain-types): add decode helper utils

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): fix type

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add clone

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): fix type

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): check dup keys

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): fix linter

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): fix visibility and comment

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add validation test

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): key registration decoding test

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): move voting pk to its own file

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): derive debug

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): use .context

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): fix getter

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): check required keys

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): network tag of payment addr

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): linter

Signed-off-by: bkioshn <[email protected]>

---------

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): expose from_saturating (#131)

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): slot bigint conversion

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): txn index conversion

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): conversion

Signed-off-by: bkioshn <[email protected]>

* fix(rust/cardano-blockchain-types): fix CIP36 (#133)

* fix(cardano-blockchain-types): key registration

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): registration witness

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): voting pk

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): validation test

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): move cip36 to be under metadata

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): use decode_helper from cbork-utils

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): visibility, cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): handle unset data

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 err report

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): update cbor-utils tag

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 now contain validation

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 validation implement getter

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): remove validation from cip36 struct

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): improve verifying key error log

Signed-off-by: bkioshn <[email protected]>

---------

Signed-off-by: bkioshn <[email protected]>

* fix(rust/cardano-blockchain-types): implement new error report for CIP36 (#142)

* fix(cardano-blockchain-types): key registration

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): registration witness

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): voting pk

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): validation test

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): move cip36 to be under metadata

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): use decode_helper from cbork-utils

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): visibility, cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): handle unset data

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 err report

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): update cbor-utils tag

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 now contain validation

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 validation implement getter

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): remove validation from cip36 struct

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): improve verifying key error log

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): implement new error report

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): function name

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): print report

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): version for catalyst-types

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): move found key check to its own function

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): pass cip36 ProblemReport as context

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): key reg redundant code and missing key checking

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): remove cip36validation struct

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add more fileds, fix return error

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): add cip36_from_block function

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): remove serde_json

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): impl display for cip36 error

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): cip36 key reg function

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): problem report conversion err

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): impl display for cip36

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): err report getter

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): update catalyst-types tag

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): return type for cip36 constructor

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): logic

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): format

Signed-off-by: bkioshn <[email protected]>

* fix(cardano-blockchain-types): rename ctx

Signed-off-by: bkioshn <[email protected]>

---------

Signed-off-by: bkioshn <[email protected]>

---------

Signed-off-by: bkioshn <[email protected]>
Co-authored-by: Steven Johnson <[email protected]>
Co-authored-by: Steven Johnson <[email protected]>
Co-authored-by: Stanislav Tkach <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants