Update to include dynamic control of parameters #25
Conversation
The dynamics package is included to allow for controlled changes to be made to various parameters throughout the MadNet system. These changes may be desired at some point in the future. Some of the changes include allowing for fees in application objects, including transaction fees. There were also some changes to crypto/ to ensure the objects behave as expected and do not panic under certain circumstances.
| rs := b.PreCommitStepStarted | ||
| return rs+int64(constants.PreCommitStepTO)/constants.OneBillion < time.Now().Unix() | ||
| return rs+int64(preCommitStepTO)/constants.OneBillion < time.Now().Unix() |
There was a problem hiding this comment.
Instead of converting with the constants.OneBillion use the constants out of the time package to perform conversion. This makes unit analysis easier for future readers. I will not block this PR for this, but please open a ticket in the backlog to fix in the future.
| @@ -20,8 +20,8 @@ func (b *PendingHdrLeafKey) UnmarshalBinary(data []byte) error { | |||
| if len(data) != 34 { | |||
There was a problem hiding this comment.
This check is done in several files. Please write a ticket to change code such that it performs the length check against the data using the constants hashlen and the len of the prefix. This will make code more readable and less fragile under modification. This will not block merge of PR.
| @@ -7,6 +7,16 @@ import ( | |||
| to "github.com/MadBase/MadNet/proto" | |||
| ) | |||
|
|
|||
| // TODO: ForwardTranslateTXOut and ReverseTranslateTXOut need to be updated | |||
There was a problem hiding this comment.
This can be solved by generating the associated protobuf definitions in proto and importing the generated objects instead of using the objects from /application. Please open a ticket for this change. This will not block PR at this time.
There was a problem hiding this comment.
This PR is ready for merge but several standalone tasks must be completed before this code may be fully leveraged. I have commented on these points in code to highlight necessary changes. In order to get this merged, please open tickets in the icebox as described in the PR conversations.
* Add initial tests for the governance system * Fix test file * Add more unit tests * Clean up code
Scope
In this PR we enable the ability to change certain parameters within MadNet in a controlled way.
The additions include fees for individual transactions UTXOs (DataStore, ValueStore, AtomicSwap)
as well as a transaction fee (TxFee object).
At the present, all of these fees are set to zero.
Why?
We want to allow controlled updates to parameters.
In particular, it would be useful to allow for fees to include transaction fees in order to prioritize
certain transactions over others.
Todos
There is still some work involving TxFee objects in capnproto that needs to be done before
the additions will work.
Specifically, it is not possible to include TxFee objects because they are not marshaled
or unmarshaled in localrpc/.
No tests have been performed to ensure that nodes update when parameter values
are changed dynamically.
These tests will need to be performed before the system goes online.