Cardano: Add vote delegation to node api #89

RostarMarek · 2024-09-27T09:22:21Z

Added vote delegation to node api and wasm generation.

I still have the issue where I can't build that I mentioned here so I wasn't able to load firmware to my dev device to properly test it out.

refi93 · 2024-10-04T11:51:42Z

src/wasm/types.rs

+  | {
+      voteDelegation: {
+        keypath: Keypath
+        type: number


number is a bit too generic, wouldn't it be possible to narrow the values down with some enum? That would potentially allow for making this a union of different object shapes, making it clear when is drep hash required and when it isn't

Moreover, if possible, instead of null, I'd consider making the drepCredHash optional, at least from a typescript perspective it would feel more "natural"

I agree, this type should be something like

CardanoDRepType = 'keyHash' | 'scriptHash' | 'alwaysAbstain' | 'alwaysNoConfidence'

if it doesn't work out of the box like this, check other enums (e.g. in btc.proto/btc.rs) and scripts/build-protos.rs for reference,

benma · 2024-10-10T06:04:15Z

src/wasm/types.rs

+  | {
+      voteDelegation: {
+        keypath: Keypath
+        type: number


I agree, this type should be something like

CardanoDRepType = 'keyHash' | 'scriptHash' | 'alwaysAbstain' | 'alwaysNoConfidence'

if it doesn't work out of the box like this, check other enums (e.g. in btc.proto/btc.rs) and scripts/build-protos.rs for reference,

benma · 2024-10-10T06:05:32Z

src/wasm/types.rs

+      voteDelegation: {
+        keypath: Keypath
+        type: number
+        drepCredhash: Uint8Array | undefined | null


why undefined or null? Seems like drepCredhash?: Uint8Array should be enough.

in scripts/build-protos.rs, you could rename this to drepCredHash (proper camelCase)

benma · 2024-10-10T06:08:22Z

sandbox/src/Cardano.tsx

 }

-export function Cardano({ bb02 } : Props) {


please keep the same style (spaces, indent level, ...)

benma

Please also add a CHANGELOG in CHANGELOG-npm.md under 0.7.0

If you rebase, the failing nightly CI item should disappear.

benma · 2024-10-28T11:14:54Z

scripts/build-protos.rs

+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.KEY_HASH",
+            "serde(rename = \"keyHash\")",
+        ),
+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.SCRIPT_HASH",
+            "serde(rename = \"scriptHash\")",
+        ),
+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.ALWAYS_ABSTAIN",
+            "serde(rename = \"alwaysAbstain\")",
+        ),
+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.ALWAYS_NO_CONFIDENCE",
+            "serde(rename = \"alwaysNoConfidence\")",
+        ),


These are not needed because camelCase is the default - the dRepType enum has this attribute already:

#[cfg_attr(feature = "wasm", serde(rename_all = "camelCase"))]

Could you remove these?

I think you missed make build-protos afterwards, as these tags are still in the generated protobuf file. Please run make build-protos again.

benma · 2024-10-28T11:18:26Z

src/wasm/types.rs

+      voteDelegation: {
+        keypath: Keypath
+        type: CardanoDrepType
+        drepCredHash?: Uint8Array


you changed the name here but the deserialization still uses drepCredhash, so if you tried to use drepCredHash, it would not actually show the hash and error after confirmation.

benma · 2024-10-28T11:19:45Z

sandbox/src/Cardano.tsx

@@ -151,6 +150,7 @@ function CardanoSignTransaction({ bb02 }: Props) {
    ['zero-ttl', 'Transaction with TTL=0'],
    ['tokens', 'Transaction sending tokens'],
    ['delegate', 'Delegate staking to a pool'],
+    ['vote-delegation', 'Delegate vote to a dRep'],


Please add another one that delegates to a keyHash or scriptHash, or add a dropdown to the existing one to switch the mode so all of them can be tested in the sandbox. The hash ones don't work right now (see other comment).

benma

Also please add an entry to CHANGELOG-npm.md

benma · 2024-10-31T08:28:37Z

In the meantime we released v0.7.0 to npmjs. Please rebase and change NPM_VERSION to 0.8.0. Thanks.

benma · 2024-11-14T13:43:56Z

CHANGELOG-npm.md

@@ -2,6 +2,9 @@

 ## Unreleased

+## 0.8.0


please also change NPM_VERSION - https://github.com/BitBoxSwiss/bitbox-api-rs/blob/master/NPM_VERSION

benma · 2024-11-14T13:45:17Z

scripts/build-protos.rs

+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.KEY_HASH",
+            "serde(rename = \"keyHash\")",
+        ),
+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.SCRIPT_HASH",
+            "serde(rename = \"scriptHash\")",
+        ),
+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.ALWAYS_ABSTAIN",
+            "serde(rename = \"alwaysAbstain\")",
+        ),
+        (
+            "shiftcrypto.bitbox02.CardanoSignTransactionRequest.Certificate.VoteDelegation.CardanoDRepType.ALWAYS_NO_CONFIDENCE",
+            "serde(rename = \"alwaysNoConfidence\")",
+        ),


I think you missed make build-protos afterwards, as these tags are still in the generated protobuf file. Please run make build-protos again.

Added vote delegation to node api and wasm generation. Signed-off-by: RostarMarek <[email protected]>

benma

Thanks 🎉

RostarMarek force-pushed the rostarmarek/cardano/npm-vote-delegation branch from 22aaf65 to abcd7f4 Compare September 27, 2024 11:08

RostarMarek marked this pull request as ready for review September 27, 2024 12:12

refi93 reviewed Oct 4, 2024

View reviewed changes

benma requested changes Oct 10, 2024

View reviewed changes

benma reviewed Oct 10, 2024

View reviewed changes

RostarMarek force-pushed the rostarmarek/cardano/npm-vote-delegation branch 3 times, most recently from 00d5d6a to eb4173a Compare October 11, 2024 11:20

benma requested changes Oct 28, 2024

View reviewed changes

RostarMarek force-pushed the rostarmarek/cardano/npm-vote-delegation branch 2 times, most recently from f211ed6 to 18b8237 Compare November 12, 2024 00:33

RostarMarek requested a review from benma November 12, 2024 10:42

benma requested changes Nov 14, 2024

View reviewed changes

Cardano: Add vote delegation to node api

afe26fd

Added vote delegation to node api and wasm generation. Signed-off-by: RostarMarek <[email protected]>

RostarMarek force-pushed the rostarmarek/cardano/npm-vote-delegation branch from 18b8237 to afe26fd Compare November 18, 2024 12:05

benma approved these changes Nov 18, 2024

View reviewed changes

benma merged commit 49e5ab8 into BitBoxSwiss:master Nov 18, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cardano: Add vote delegation to node api #89

Cardano: Add vote delegation to node api #89

RostarMarek commented Sep 27, 2024

refi93 Oct 4, 2024

benma Oct 10, 2024

benma Oct 10, 2024

benma Oct 10, 2024

benma Oct 10, 2024

benma left a comment •

edited

Loading

benma Oct 28, 2024

benma Nov 14, 2024 •

edited

Loading

benma Oct 28, 2024 •

edited

Loading

benma Oct 28, 2024

benma left a comment •

edited

Loading

benma commented Oct 31, 2024

benma Nov 14, 2024 •

edited

Loading

benma Nov 14, 2024 •

edited

Loading

benma left a comment

Cardano: Add vote delegation to node api #89

Cardano: Add vote delegation to node api #89

Conversation

RostarMarek commented Sep 27, 2024

refi93 Oct 4, 2024

Choose a reason for hiding this comment

benma Oct 10, 2024

Choose a reason for hiding this comment

benma Oct 10, 2024

Choose a reason for hiding this comment

benma Oct 10, 2024

Choose a reason for hiding this comment

benma Oct 10, 2024

Choose a reason for hiding this comment

benma left a comment • edited Loading

Choose a reason for hiding this comment

benma Oct 28, 2024

Choose a reason for hiding this comment

benma Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

benma Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

benma Oct 28, 2024

Choose a reason for hiding this comment

benma left a comment • edited Loading

Choose a reason for hiding this comment

benma commented Oct 31, 2024

benma Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

benma Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

benma left a comment

Choose a reason for hiding this comment

benma left a comment •

edited

Loading

benma Nov 14, 2024 •

edited

Loading

benma Oct 28, 2024 •

edited

Loading

benma left a comment •

edited

Loading

benma Nov 14, 2024 •

edited

Loading

benma Nov 14, 2024 •

edited

Loading