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

Pallas signer stake delegation #464

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Sventimir
Copy link

Motivation

The following PR updates the pallas signer used for mina.

  • It adds support for stake delegation transactions, in order to test the mina rosetta implementation.
  • It fixes the default value for the validUntil field of payment transaction (from 0 to max_uint32).
    This was tested by running the construction tests on the mina rosetta server with both payment and delegation transactions.

Solution

Delegation transactions

The Transaction type could already handle various types of transaction and distinguish between them using the Tag field.
{false, false, false} is the first case (payments) and {false,false,true} the second one (stake delegation).
However the parsing step in signer_pallas.go could only handle Payment transaction so far so I added the StakeDelelegation case.

The valid_until field

If the valid_until field is not specified in the preprocess step, the rosetta server uses the greatest uint32 as a default value. The pallas signer of rosetta-sdk-go was defaulting to zero when building the signing payload which resulted in an invalid signature error. The greatest uint32 is now used in both cases.

@Sventimir
Copy link
Author

Sventimir commented Jan 3, 2023

This is the duplicate of #457 , which should be closed in favour of this PR. The reason for this is that @ylecornec no longer works on it and we won't be able to make changes to his PR if so requested.

@Sventimir Sventimir force-pushed the pallas_signer_stake_delegation branch from e9d9f74 to 0e6a446 Compare February 7, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants