Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remarks amendment #301

Open
wants to merge 33 commits into
base: dev
Choose a base branch
from
Open

Remarks amendment #301

wants to merge 33 commits into from

Conversation

RichardAH
Copy link
Contributor

@RichardAH RichardAH commented Mar 25, 2024

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@RichardAH RichardAH changed the title featureRemarks [DO NOT MERGE] Remarks amendment [DO NOT MERGE] Mar 25, 2024
@RichardAH RichardAH changed the title Remarks amendment [DO NOT MERGE] Remarks amendment Mar 27, 2024

namespace ripple {
namespace test {
struct SetRemarks_test : public beast::unit_test::suite
Copy link
Contributor Author

Choose a reason for hiding this comment

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

when these tests are finished I'm probably ready to merge

@dangell7 dangell7 added Amendment New feature or fix amendment feature Feature Request labels Sep 19, 2024
@RichardAH RichardAH force-pushed the dev branch 2 times, most recently from 77b8c83 to 532a471 Compare December 11, 2024 02:30
Comment on lines +121 to +125
if (tx.isFieldPresent(sfFlags) && tx.getFieldU32(sfFlags) != 0)
{
JLOG(j.warn()) << "SetRemarks: Invalid flags set.";
return temMALFORMED;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

should only allow lsfImmutable?

And temINVALID_FLAG is better thantemMALFORMED here

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Amendment New feature or fix amendment feature Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants