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

Association History Processing (Part 2) #618

Merged
merged 1 commit into from
Apr 6, 2024

Conversation

neekolas
Copy link
Contributor

@neekolas neekolas commented Apr 4, 2024

Summary

  • Adds the actual association log and verifications
  • Adds a bunch of tests

TODO

  • More tests coming
  • Verify all logic in the XIP is identical to the implementation
  • Serialization and deserialization
  • Actual signature types

Copy link
Contributor Author

neekolas commented Apr 4, 2024

@neekolas neekolas changed the title added association log and signature Add Association History and IdentityUpdate processing Apr 4, 2024
@neekolas neekolas changed the title Add Association History and IdentityUpdate processing Association History Processing (Part 2) Apr 4, 2024
@neekolas neekolas force-pushed the 04-04-association_history_processing_pt._2 branch 7 times, most recently from 7c48f72 to 1068a6b Compare April 5, 2024 15:41
@neekolas neekolas marked this pull request as ready for review April 5, 2024 16:56
@neekolas neekolas requested a review from a team as a code owner April 5, 2024 16:56
Copy link
Contributor

@insipx insipx left a comment

Choose a reason for hiding this comment

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

This seems good to me -- in an ideal world i'd like to see less boxing, but I like the builder-esque strategy of the IdentityAction interface for the log actions.

}

#[derive(Clone, Debug, PartialEq)]
pub enum SignatureKind {
Copy link
Contributor

Choose a reason for hiding this comment

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

does it make sense to handle an Other(String) kind?

For instance, in the case that someone wants to implement their custom signature type, or would that break too many assumptions?

pub struct CreateInbox {
pub nonce: u64,
pub account_address: String,
pub initial_address_signature: Box<dyn Signature>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we boxing all these Signatures to make it easier for ffi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mostly out of expedience, although I think it does make FFI easier.

If we can come up with an unboxed solution that works everywhere I'm all for it.

Copy link
Contributor Author

neekolas commented Apr 6, 2024

Merge activity

  • Apr 5, 8:03 PM EDT: @neekolas started a stack merge that includes this pull request via Graphite.
  • Apr 5, 8:04 PM EDT: Graphite rebased this pull request as part of a merge.
  • Apr 5, 8:09 PM EDT: @neekolas merged this pull request with Graphite.

Base automatically changed from 04-04-association_history_processing_pt._1 to main April 6, 2024 00:03
@neekolas neekolas force-pushed the 04-04-association_history_processing_pt._2 branch from 1068a6b to 095ddb7 Compare April 6, 2024 00:04
@neekolas neekolas merged commit c4150df into main Apr 6, 2024
9 checks passed
@neekolas neekolas deleted the 04-04-association_history_processing_pt._2 branch April 6, 2024 00:09
@37ng 37ng mentioned this pull request Apr 10, 2024
@neekolas neekolas mentioned this pull request Apr 10, 2024
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.

3 participants