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

UIP-0027 - Independent service fields for transaction #21

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ protocol and APIs.
|[UIP-0021](https://github.com/dtr-org/uips/blob/master/UIP-0021.md)|Transfer Esperanza Transactions|Proposed|2018-11-08|
|[UIP-0024](https://github.com/dtr-org/uips/blob/master/UIP-0024.md)|CTOR - Canonical Transactions Ordering Rule|Proposed|2018-12-14|
|[UIP-0026](https://github.com/dtr-org/uips/blob/master/UIP-0026.md)|Graphene - block propagation protocol|Proposed|2019-02-21|
|[UIP-0027](https://github.com/dtr-org/uips/blob/master/UIP-0027.md)|Transaction service fields|Draft|2019-05-07|

The team is committed to fostering a welcoming and harassment-free
environment. All participants are expected to adhere to our [code of
Expand Down
93 changes: 93 additions & 0 deletions UIP-0027.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# UIP-27: Transaction service fields

```
Author: Dima Saveliev <[email protected]>
Status: Draft
Created: 2019-05-07
```

## Abstract

This document describes protocol changes in the transaction structure:
the `nVersion` field is going to be replaced by four separate service fields:
* type
* version
* two more fields are vacant


## Motivation

[UIP-0018](https://github.com/dtr-org/uips/blob/master/UIP-0018.md) introduced **a transaction type** concept
and added the corresponding field as a part of already existing `nVersion` field.
That was achieved by overloading `nVersion` with bit arithmetic at the cost of increasing complexity of the code.

By implementing separate service fields we solve the following problems:

1. Clean-up the logic behind the `type` and `version` fields.
2. Minimize the size of `type` and `version` fields.
3. Minimize the impact on the existing test suite.

As an additional benefit, we will have two more fields for possible future needs.


## Specification

At the moment we have this `nVersion` field structure:

_These pictures illustrate `nVersion` in big-endian order as we have it in the runtime.
It's worth noting, however, that the serialized bytes will have a little-endian order._

```
0x00 0x00 0x00 0x00
^------^ ^------^ ^----------------^
unused type version
^------------------------------------^
nVersion
```

* `nVersion` - original `uint32_t` value
* `type` - derived `uint8_t` attribute
* `version` - derived `uint16_t` attribute

Proposed layout for the same bytes:
```
0x00 0x00 0x00 0x00
^------^ ^------^ ^------^ ^------^
reserved type reserved version
dsaveliev marked this conversation as resolved.
Show resolved Hide resolved
```

* `type` - independent`uint8_t` field
* `version` - independent`uint8_t` field


## Rationale
Copy link
Member

@frolosofsky frolosofsky May 10, 2019

Choose a reason for hiding this comment

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

In both Rationale and Motivation parts, I did not find a description of a real problem we're trying to solve. The essential proposed change is literally to shrink version from 16 bits to 8 bits that leads to having two dead bytes instead of one.

Personally, I'd rather replace nVersion (4 bytes) with two fields: version and txtype (each 1 byte). As for bitcoin tests which relies on "real" transactions, I think we can get rid of them in favor of new tests for unit-e transactions. Having this compatibility with bitcoin limits us in further transactions manipulations.

If we say that it's too hard to achieve and maintain (which is understandable), then I don't see a point in this UIP. We can pinch unused byte of nVersion anytime we need, as we already did it in UIP-18.


This format was chosen in order to preserve the existing byte order and to minimize the impact on the existing test suite.
It's very hard to add, remove or even swap existing bytes due to a couple of reasons:

* Tests contain a lot of magic numbers, based on the assumptions regarding transaction size.
* Some tests heavily rely on hardcoded, unique transactions from the real Bitcoin network.
* Unit-e wants to sync regularly with Bitcoin Core with minimal effort.

All these conditions make structural changes hard, thus it's much more effective
to act in the context of the existing layout.

By implementing four separate fields we decouple unrelated entities, remove technical debt
without a tremendous amount of extra work and keep our test suite intact hence eliminate some range of security risks.


## Backwards compatibility

The proposed change will have a minimal impact on existing Unit-e codebase and will not change the byte order of existing messages.
We shrink the actual size of `version` field down to 1 byte, but it's not a problem, because the range
of [0, 255] integer values is more than enough to reflect existing transaction versions.

## Reference implementation

Work in progress.

## Copyright

This document and all its auxiliary files are dual-licensed under
[CC0](https://creativecommons.org/publicdomain/zero/1.0/) and
[MIT](https://opensource.org/licenses/MIT).