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

Known Amendments updates for 1.9.x #1409

Merged
merged 5 commits into from
May 24, 2022
Merged

Known Amendments updates for 1.9.x #1409

merged 5 commits into from
May 24, 2022

Conversation

mDuo13
Copy link
Collaborator

@mDuo13 mDuo13 commented May 20, 2022

  • Add ExpandedSignerList, fixNFTokenDirV1, NonFungibleTokensV1
  • Correct some other minor issues with the Known Amendments page
  • Add the new amendments to the snippet for automatic linking
  • Update multisigning pages for ExpandedSignerList
  • Fix some broken links

@github-actions
Copy link

Link check report. 389935 links checked.
Success! No broken links found.

Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/ka_191/

Style Report

1 similar comment
@github-actions
Copy link

Link check report. 389935 links checked.
Success! No broken links found.

Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/ka_191/

Style Report

@github-actions
Copy link

Link check report. 389942 links checked.
Success! No broken links found.

Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/ka_191/

Style Report

1 similar comment
@github-actions
Copy link

Link check report. 389942 links checked.
Success! No broken links found.

Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/ka_191/

Style Report

Copy link
Collaborator

@amarantha-k amarantha-k left a comment

Choose a reason for hiding this comment

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

Other than a few minor suggestions. lgtm.

Co-authored-by: Amarantha Kulkarni <[email protected]>
Co-authored-by: Elliot Lee <[email protected]>
@mDuo13 mDuo13 merged commit fef2a62 into master May 24, 2022
@mDuo13 mDuo13 deleted the ka_191 branch May 24, 2022 00:34
@github-actions
Copy link

Link check report. 390003 links checked.
Success! No broken links found.

Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/ka_191/

Style Report

Copy link
Contributor

@scottschurr scottschurr left a comment

Choose a reason for hiding this comment

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

Looks good, but I did leave a few suggestions that might be improvements. Your call.

| B2A4DB846F0891BF2C76AB2F2ACC8F5B4EC64437135C6E56F3F859DE5FFD5856 | 投票中 |

<!-- TODO: translate description -->
This amendment expands the maximum signer list size allows each signer to have optional data associated with it. The additional data can be used to identify the signer, which may be useful for smart contracts, or for identifying who controls a key in a large organization: for example, you could store an IPv6 address or the identifier of a Hardware Security Module (HSM).
Copy link
Contributor

Choose a reason for hiding this comment

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

... maximum signer list size and allows ...?

| 0285B7E5E08E1A8E4C15636F0591D87F73CB6A7B6452A932AD72BBC8E5D1CBE3 | 投票中 |

<!-- TODO: translate description -->
This amendment fixes an off-by-one error that occurred in some corner cases when determining which `NFTokenPage` an `NFToken` object belongs on. It also adjusts the constraints of `NFTokenPage` invariant checks, so that certain error cases fail with a suitable error code such as `tecNO_SUITABLE_TOKEN_PAGE` instead of failing with a `tecINVARIANT_FAILED` error code.
Copy link
Contributor

Choose a reason for hiding this comment

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

The second sentence is not quite right. Perhaps something like this?

This amendment fixes errors that occurred in some corner cases when determining which NFTokenPage an NFToken object belongs on. Prior to the fix the errors would cause a tecINVARIANT_FAILED error code. With the fixes either no error occurs, or a more suitable error code is generated such as tecNO_SUITABLE_TOKEN_PAGE. The invariant checking for NFTokenPage is also expanded yet further to cover more possible error conditions.

| Default Vote (Latest stable release) | No |
| Pre-amendment functionality retired? | No |

This amendment expands the maximum signer list size allows each signer to have optional data associated with it. The additional data can be used to identify the signer, which may be useful for smart contracts, or for identifying who controls a key in a large organization: for example, you could store an IPv6 address or the identifier of a Hardware Security Module (HSM).
Copy link
Contributor

Choose a reason for hiding this comment

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

... maximum signer list size and allows ...?

| Default Vote (Latest stable release) | No |
| Pre-amendment functionality retired? | No |

This amendment fixes an off-by-one error that occurred in some corner cases when determining which `NFTokenPage` an `NFToken` object belongs on. It also adjusts the constraints of `NFTokenPage` invariant checks, so that certain error cases fail with a suitable error code such as `tecNO_SUITABLE_TOKEN_PAGE` instead of failing with a `tecINVARIANT_FAILED` error code.
Copy link
Contributor

Choose a reason for hiding this comment

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

The second sentence is not quite right. Perhaps something like this?

This amendment fixes errors that occurred in some corner cases when determining which NFTokenPage an NFToken object belongs on. Prior to the fix the errors would cause a tecINVARIANT_FAILED error code. With the fixes either no error occurs, or a more suitable error code is generated such as tecNO_SUITABLE_TOKEN_PAGE. The invariant checking for NFTokenPage is also expanded yet further to cover more possible error conditions.

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.

4 participants