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 #457

Closed

Conversation

ylecornec
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.

@ylecornec ylecornec marked this pull request as ready for review December 5, 2022 14:15
@ylecornec ylecornec force-pushed the pallas_signer_stake_delegation branch from 805a176 to e9d9f74 Compare December 13, 2022 10:48
@ylecornec
Copy link
Author

The tests should now be fixed (make test succeeds locally) and I added delegation versions of existing payment tests.

@ylecornec
Copy link
Author

Closing in favor of #464

@ylecornec ylecornec closed this Jan 3, 2023
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.

1 participant