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

IF: Use string for variant format of fc::dynamic_bitset #67

Merged
merged 6 commits into from
Apr 25, 2024

Conversation

heifner
Copy link
Member

@heifner heifner commented Apr 24, 2024

Original format in the recorded IBC demo:

"qc_extension": {
  "qc": {
     "block_num": 135,
     "qc": {
       "_strong_votes": [
         15
       ],
       "_sig": "SIG_BLS_xxx"
    }
  }
}

Format before this PR:

  "qc_extension": {
    "qc": {
      "block_num": 135,
      "qc": {
        "_strong_votes": {
          "size": 5,
          "bits": [
            15
          ]
        },
        "_sig": "SIG_BLS_xxx"
      }
    }
  },

Format in this PR:
A character in the string is '1' if the corresponding bit is set, and '0' if it is not. Character position i in the string corresponds to bit position b.size() - 1 - i.

  "qc_extension": {
    "qc": {
      "block_num": 135,
      "qc": {
        "_strong_votes": "01111",
        "_sig": "SIG_BLS_xxx"
      }
    }
  },

Also rename some reflected types so their JSON name does not have a _ prefix.

@heifner heifner requested review from greg7mdp and linh2931 April 24, 2024 17:30
@heifner heifner added the OCI Work exclusive to OCI team label Apr 24, 2024
@greg7mdp
Copy link
Contributor

Format before this PR:

why is "size": 5. I thought there were 3 finalizers?
why "bits": [ 31 ]. I would have expected the same 7 as before

Format in this PR:

7 should translate to "_strong_votes": "111",

@heifner
Copy link
Member Author

heifner commented Apr 24, 2024

Format before this PR:

why is "size": 5. I thought there were 3 finalizers? why "bits": [ 31 ]. I would have expected the same 7 as before

Format in this PR:

7 should translate to "_strong_votes": "111",

It is not the same number of finalizers in the examples.

@greg7mdp
Copy link
Contributor

It is not the same number of finalizers in the examples.

The examples illustrating the changes by this PR should use the same data for clarity I think.

@heifner
Copy link
Member Author

heifner commented Apr 24, 2024

It is not the same number of finalizers in the examples.

The examples illustrating the changes by this PR should use the same data for clarity I think.

Updated the PR description to use the same example.

greg7mdp
greg7mdp previously approved these changes Apr 24, 2024
linh2931
linh2931 previously approved these changes Apr 24, 2024
@heifner heifner requested review from greg7mdp and linh2931 April 24, 2024 23:00
@heifner heifner dismissed stale reviews from linh2931 and greg7mdp April 24, 2024 23:00

Implementation has significantly changed.

@heifner heifner merged commit ef37672 into GH-13-disaster-test Apr 25, 2024
36 checks passed
@heifner heifner deleted the dynamic_bitset_variant_format branch April 25, 2024 11:18
@ericpassmore
Copy link
Contributor

Note:start
group: IF
category: INTERNALS
summary: Update representation of variant formate for fc::dynamic_bitset to make votes more readable.
Note:end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants