Implement MultiSignReserve amendment [RIPD-1647]: #2649
Conversation
Jenkins Build SummaryBuilt from this commit Built at 20181002 - 06:45:44 Test Results
|
| int | ||
| SetSignerList::ownerCountDelta (std::size_t entryCount) | ||
| SetSignerList::oldOwnerCountDelta (std::size_t entryCount) |
There was a problem hiding this comment.
Calling functions old... or new... get awkward if we modify this code again. Sometimes during a transition we can name things old... and eventually remove the function, but this function needs to stay around forever. For a function like this, I'm OK with a long cumbersome name if you can think of one. Maybe multiOwnerCountDelta?
There was a problem hiding this comment.
preMultiSignReserveAmendmentOwnerCountDelta? ;-)
There was a problem hiding this comment.
Great point about the "old" name. I struggled with this a bit and then punted. Maybe signerCountBasedOwnerCountDelta()? That reflects what the function does and what we moved away from. I'll go with that unless I hear objections or a better suggestion.
| @@ -152,6 +152,9 @@ enum LedgerSpecificFlags | |||
| lsfHighNoRipple = 0x00200000, | |||
| lsfLowFreeze = 0x00400000, // True, low side has set freeze flag | |||
| lsfHighFreeze = 0x00800000, // True, high side has set freeze flag | |||
|
|
|||
| // ltSIGNER_LIST | |||
| lsfOneOwnerCount = 0x00010000, // True, uses only one OwnerCount | |||
There was a problem hiding this comment.
I wish we didn't have to do this, but I don't see another way.
scottschurr
force-pushed
the
multisig-reserve
branch
from
b181473
to
4756680
Compare
|
Fixed name and rebased. Thanks for the review, @seelabs! |
| ledgerEntry->setFieldU32 (sfSignerListID, defaultSignerListID_); | ||
| if (flags) // If the flags are zero default them (they default to zeroed). |
There was a problem hiding this comment.
The comment is a little weirdly worded and I’m not sure it really adds a lot. I think that “non-default flags specified” works better.
There was a problem hiding this comment.
Good point. I've revised the wording on this comment. Thanks.
Codecov Report
@@ Coverage Diff @@
## develop #2649 +/- ##
===========================================
- Coverage 69.96% 69.96% -0.01%
===========================================
Files 702 702
Lines 55141 55151 +10
===========================================
+ Hits 38578 38584 +6
- Misses 16563 16567 +4
Continue to review full report at Codecov.
|
scottschurr
added
Tx Change
Passed
scottschurr
force-pushed
the
multisig-reserve
branch
from
21561eb
to
3c18b92
Compare
|
Squashed and rebased to 1.1.0-rc3. |
scottschurr
force-pushed
the
multisig-reserve
branch
from
3c18b92
to
6c27a79
Compare
Reduces the account reserve for a multisigning SignerList from (conditionally) 3 to 10 OwnerCounts to (unconditionally) 1 OwnerCount. Includes a transition process.
scottschurr
force-pushed
the
multisig-reserve
branch
from
6c27a79
to
6572fc8
Compare
|
Rebased to 1.2.0-b2. |
|
In 1.2.0-b3 |
Reduces the account reserve for a multisigning SignerList:
Includes a transition process. Implements the changes described in the internal spec.
No hurry on the review. We probably don't want to merge this until after 1.1.0 is in the bag.