RFC: Transaction type #3

samuel-rufi · 2019-09-06T21:13:18Z

text/0000-bee-model-crate/0000-bee-model-crate.md

Alex6323 · 2019-09-26T22:15:26Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+
+# Summary
+
+This feature is responsible for the creation and interpretation of transactions and bundles.


I think I'ld like the Summary a little bit more detailed, what is actually introduced by this feature. I know that transactions and bundles are absolutely essential to IOTA, but for consistency with other RFCs could you write this section "as if" you would introduce those concepts with this proposal. I think we can expect the reader to have "some" DLT knowledge (so this summary should still remain rather short), but IMHO we should write those RFCs also for people that are fairly new to IOTA to get as many people involved as possible. The summary/abstract will be the first thing someone interested in it will read, and most of the times the last thing as well.

Alex6323 · 2019-09-26T22:45:37Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+IOTA is a distributed ledger that was designed for payment settlement and data transfer between machines and devices in the Internet of Things (IoT) ecosystem.
+The data packets that are sent through the network are called "transactions".
+Settlements or data transfers can be done with the help of these transactions. Payment settlements require the help of multiple transactions.
+


I think those sentences should move into the summary as they very well provide the necessary context to understand this proposal. To not repeat yourself in this section, you could pick up from the summary and elaborate on what transactions and bundles are in more detail, and how they are useful or even indispensable for the Bee framework.

I agree with @Alex6323 here.

text/0000-bee-model-crate/0000-bee-model-crate.md

Updated the sentence how transactions can be built. Removed tx hash from Transaction struct.

SuperFluffy

I like this RFC because I understand its purpose, and because I feel like it fulfills (or is very close to fulfilling) the requirement of taking care of a specific, cleanly separated concern.

The proposal should definitely separate the Transaction and Bundle types into their respective RFCs, because these seem to be sufficiently different.

The RFC can mention that the Transaction type could live in its own crate.

text/0000-bee-model-crate/0000-bee-model-crate.md

+
+# Summary
+
+This feature is responsible for the creation and interpretation of transactions and bundles.


SuperFluffy · 2019-10-01T13:42:10Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+IOTA is a distributed ledger that was designed for payment settlement and data transfer between machines and devices in the Internet of Things (IoT) ecosystem.
+The data packets that are sent through the network are called "transactions".
+Settlements or data transfers can be done with the help of these transactions. Payment settlements require the help of multiple transactions.
+


I agree with @Alex6323 here.

text/0000-bee-model-crate/0000-bee-model-crate.md

SuperFluffy · 2019-10-01T14:10:51Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+
+# Drawbacks
+
+Without any bee-model crate, nodes can not exchange transactions. Therefore this crate seems necessary.


Drawbacks should ideally also discuss the drawbacks of this design. :)

I am thinking of the String types, for example, which could also be done as their own types.

text/0000-bee-model-crate/0000-bee-model-crate.md

SuperFluffy · 2019-10-01T14:13:21Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+- The distinction between Transaction and Transaction Builder as well as Bundle and Bundle Builder makes the code cleaner 
+and helps achieve correctness among the data objects. Properties are clearly assigned to specific data objects and not mixed up.
+
+- The proposed crate interface is intuitive. Completely different alternatives did not naturally come to mind.


If it's intuitive is for users to decide. I agree however that it is a very natural proposition to basically treat the incoming transaction as some encoded binary blob.

text/0000-bee-model-crate/0000-bee-model-crate.md

SuperFluffy · 2019-10-01T14:16:07Z

text/0000-bee-model-crate/0000-bee-model-crate.md

@@ -0,0 +1,334 @@
+ Feature Name: `bee-model-crate`


We should rename it to something like iri-transaction-model or mainnet-transaction-model or v1-transaction-model.

I'd vote for versioning since mainnet-transaction-model is not time-proof and iri-transaction-model is not really the case.

How do you want to version it?

text/0000-bee-model-crate/0000-bee-model-crate.md

thibault-martinez · 2019-10-01T16:15:32Z

text/0000-bee-model-crate/0000-bee-model-crate.md

@@ -0,0 +1,334 @@
+ Feature Name: `bee-model-crate`


I'd vote for versioning since mainnet-transaction-model is not time-proof and iri-transaction-model is not really the case.

text/0000-bee-model-crate/0000-bee-model-crate.md

thibault-martinez · 2019-10-01T16:25:31Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+```rust
+struct Transaction {
+
+    signature_fragments: String,


I'd like the name to reflect that it doesn't necessarily contain a signature. signature_or_message ?

Is it possible to even not contain this field if the instance doesn't have signature_or_message? It would be sth like this:

Suggested change

signature_fragments: String,

signature_or_message: Option<String>,

Option still allocates memory for the field so it's not really helpful

@SuperFluffy what are your thoughts regarding Option. Semantically, it would be more correct, right?

I'm not sure it makes more sense "semantically", an empty sig_or_msg doesn't mean there is no sig_or_msg. There is always a sig_or_msg :)

Agree, so maybe Option isn't necessary. And if we use static types, we could initialize it with an empty (9's) SignatureOrMessageFragment and could be sure the internal state of the builder object is always valid.

text/0000-bee-model-crate/0000-bee-model-crate.md

Co-Authored-By: Thibault Martinez <[email protected]>

text/0000-bee-model-crate/0000-bee-model-crate.md

…able, added unresolved question, removed Bundle part since its better to have it as separate RFC.

text/0000-bee-model-crate/0000-bee-model-crate.md

Co-Authored-By: Thibault Martinez <[email protected]>

lunfardo314 · 2019-10-02T14:59:20Z

Transaction and related types better be abstract traits since the beginning.
I refer to requirements of Q-nodes to Bee: #6 (comment) .
The assemblies would need multiple types of transactions in the same bundle and some new constrains on the ledger, other than usual constraints on token accounts. For example, there's a need to support unforkable chains, signed by the assembly.

Co-Authored-By: Wu Yu Wei <[email protected]>

…n't needed anymore, removed it from the build() of TransactionBuilder. Changed visibility of fields in TransactionBuilder to private. Setter/getter functions will be used to set/access fields. Moved from_bytes() constructor of Transaction to TransactionBuilder. TransactionBuilder also allows to set Timestamp and Nonce manually in case they are available.

…getter.

text/0000-bee-model-crate/0000-bee-model-crate.md

…transaction is a tuple", and fixed typo

…d info regarding the Pow:compute(&TransactionBuilder) call since it should not be overseen and reviewed.

wusyong · 2019-10-09T04:42:20Z

text/0000-bee-model-crate/0000-bee-model-crate.md


- Introducing a fluent API to the TransactionBuilder?
+- Should we use Option<T> for the fields in TransactionBuilder? 


Some fields would need this I guess. Others like timestamp, index etc are mandatory.

The build method is responsible for ensuring that mandatory fields are set, returning Result::Err if not. The type of the field is Option if there is no natural (read: meaningful) way of providing a default.

wusyong · 2019-10-14T14:30:13Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+| `signature_or_message_fragment`   | contains the signature of the transfer                 |                 |
+|                                   | or user-defined message data                           | 6561            |


I think it's no need to be separate line. Besides, this would become totally different row. Normal markdown display would just adjust them.

Suggested change

| `signature_or_message_fragment` | contains the signature of the transfer | |

| | or user-defined message data | 6561 |

| `signature_or_message_fragment` | contains the signature of the transfer or user-defined message data | 6561 |

@SuperFluffy I think we could ignore max chars limit during table format. We should make representation correct at least.

wusyong · 2019-10-14T14:30:23Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+| `address`                         | receiver (output) if value > 0,
+|                                   | or sender (input) if value < 0                         | 243             |


SuperFluffy · 2019-10-14T14:34:50Z

Yeah, I wanted to stay within the number of max chars per line we discussed. I thought GitHub markdown supports multiline table cells, but I either chose the wrong table format or GitHub doesn't support these kinds of raw tables as pandoc does.

…

On Mon, Oct 14, 2019, 4:30 PM Wu Yu Wei ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In text/0000-bee-model-crate/0000-bee-model-crate.md <#3 (comment)>: > +| `signature_or_message_fragment` | contains the signature of the transfer | | +| | or user-defined message data | 6561 | I think it's no need to be separate line. Besides, this would become totally different row. Normal markdown display would just adjust them. ⬇️ Suggested change -| | or user-defined message data | 6561 | +| `signature_or_message_fragment` | contains the signature of the transfer or user-defined message data | 6561 | — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3?email_source=notifications&email_token=AAFLF6PY3XCAV3PIT2664J3QOR67NA5CNFSM4IUNFOQKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCH3CK2I#pullrequestreview-301344105>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFLF6PUP6JTAXDSMFLQ2N3QOR67NANCNFSM4IUNFOQA> .

wusyong · 2019-10-14T14:46:39Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+The `TransactionError` is not fully fleshed out yet. For the time being, it
+definitely contains `io::Error`:
+
+```rust
+pub enum TransactionError {
+    Io(io::Error),
+}


I think we should also start considering which error handling library to use. I saw there's another new one called thiserror. What do you think?

wusyong · 2019-10-14T14:47:34Z

text/0000-bee-model-crate/0000-bee-model-crate.md

+        unimplemented!()
+    }
+
+    pub fn address(&mut self, address: Address) -> &mut Self {


How about give a proper setter method prefix like set_address?

Yes, as RFC #344 notes it:
https://github.com/rust-lang/rfcs/blob/master/text/0344-conventions-galore.md#gettersetter-apis

SuperFluffy · 2019-10-15T12:36:55Z

Incorporated into #20.

While discussing the design of the Transaction, we realized that a new transaction is not supposed to be create directly. Instead, when constructing a new message, the Bundle is the fundamental object one wants to crate, which in turn splits up its message into several Transactions. A Transaction thus becomes a very simple type. More complicated logic should thus be handled by the Bundle.

Added RFC for implementing model crate

22f6a6b

Aligned with RFC folder structure. Little changes in code.

5f21bf8

Updating RFC with Bundle and Bundle Builder.

9d43426

tsvisabo reviewed Sep 24, 2019

View reviewed changes

text/0000-bee-model-crate/0000-bee-model-crate.md Outdated Show resolved Hide resolved

text/0000-bee-model-crate/0000-bee-model-crate.md Outdated Show resolved Hide resolved

samuel-rufi requested a review from Alex6323 September 26, 2019 11:44

Alex6323 reviewed Sep 27, 2019

View reviewed changes

samuel-rufi added 3 commits September 28, 2019 11:10

Replaced "receipt" with "receiver" and "withdrawal" with sender.

2fd4d3d

Updated the sentence how transactions can be built. Removed tx hash from Transaction struct.

Added metadata struct, and a proof of work example

b3fe519

Snapshot index fix

2bdcd48

SuperFluffy suggested changes Oct 1, 2019

View reviewed changes

thibault-martinez suggested changes Oct 1, 2019

View reviewed changes

samuel-rufi and others added 4 commits October 1, 2019 19:54

Update "Name" to "name" since template change

8195ed9

Co-Authored-By: Thibault Martinez <[email protected]>

Update "Date" to "date" since template change

da1937c

Co-Authored-By: Thibault Martinez <[email protected]>

update iotaledger/bee-rfcs#000 to #3

b6abf6a

Co-Authored-By: Thibault Martinez <[email protected]>

Order fields, fix i64 to u64, updated "Unresolved questions"

4e37c07

thibault-martinez suggested changes Oct 1, 2019

View reviewed changes

text/0000-bee-model-crate/0000-bee-model-crate.md Outdated Show resolved Hide resolved

samuel-rufi added 3 commits October 1, 2019 21:12

Fix description of last_index field. Fix unsigned/signed types

740353f

Moved motivation part to summary, displayed transaction fields in a t…

0fdbed2

…able, added unresolved question, removed Bundle part since its better to have it as separate RFC.

Enlarge RFC, Adding questions to "Unresolved questions"

7846456

thibault-martinez suggested changes Oct 2, 2019

View reviewed changes

samuel-rufi and others added 3 commits October 2, 2019 11:38

changed "built" do "build"

fdd56e5

Co-Authored-By: Thibault Martinez <[email protected]>

removed "hash suffix" in table. moved nonce after all other fields

b00dbd7

Merge remote-tracking branch 'origin/master'

7ca1c8b

samuel-rufi and others added 3 commits October 7, 2019 20:36

Update text/0000-bee-model-crate/0000-bee-model-crate.md

4a3f627

Co-Authored-By: Wu Yu Wei <[email protected]>

Changed types of TransactionMetadata and provided impl with setter & …

cc8628d

…getter.

Merge remote-tracking branch 'origin/master'

6cb0e6b

thibault-martinez suggested changes Oct 7, 2019

View reviewed changes

text/0000-bee-model-crate/0000-bee-model-crate.md Outdated Show resolved Hide resolved

added why transactions should not be modifiable, what is meant with "…

5b2fb19

…transaction is a tuple", and fixed typo

samuel-rufi changed the title ~~RFC: bee-model crate~~ RFC: bee-model module Oct 8, 2019

samuel-rufi added 2 commits October 8, 2019 23:40

Update unresolved questions.

4870bae

Add explanation for the field types. Added unresolved questions. Adde…

a9c6db7

…d info regarding the Pow:compute(&TransactionBuilder) call since it should not be overseen and reviewed.

wusyong reviewed Oct 9, 2019

View reviewed changes

wusyong changed the title ~~RFC: bee-model module~~ RFC: bee-transaction module Oct 9, 2019

Richard Janis Goldschmidt added 5 commits October 10, 2019 12:50

Simplified summary, use transaction version following IRI

fd22880

Added motivational section

7163c78

Rewrote detailed design

d4bda0d

Fix table formatting

50871c2

Simplify Transaction design

df0b8b8

samuel-rufi requested review from Alex6323, thibault-martinez, wusyong, tsvisabo, zesterer and SuperFluffy October 14, 2019 13:10

wusyong reviewed Oct 14, 2019

View reviewed changes

thibault-martinez mentioned this pull request Oct 14, 2019

RFC: IRI v1.8.1 transactions and bundles #20

Closed

thibault-martinez assigned samuel-rufi Oct 15, 2019

SuperFluffy changed the title ~~RFC: bee-transaction module~~ RFC: Transaction type Oct 15, 2019

SuperFluffy closed this Oct 15, 2019

SuperFluffy mentioned this pull request Oct 15, 2019

RFC: Bundle type #18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Transaction type #3

RFC: Transaction type #3

samuel-rufi commented Sep 6, 2019 •

edited

Loading

Alex6323 Sep 26, 2019

This comment was marked as resolved.

Alex6323 Sep 26, 2019

SuperFluffy Oct 1, 2019

SuperFluffy left a comment

This comment was marked as resolved.

SuperFluffy Oct 1, 2019

SuperFluffy Oct 1, 2019

SuperFluffy Oct 1, 2019

SuperFluffy Oct 1, 2019

thibault-martinez Oct 1, 2019

samuel-rufi Oct 1, 2019

thibault-martinez Oct 1, 2019

thibault-martinez Oct 1, 2019

wusyong Oct 3, 2019

thibault-martinez Oct 3, 2019

samuel-rufi Oct 7, 2019

thibault-martinez Oct 7, 2019

samuel-rufi Oct 7, 2019

lunfardo314 commented Oct 2, 2019

wusyong Oct 9, 2019

SuperFluffy Oct 10, 2019 •

edited

Loading

wusyong Oct 14, 2019

wusyong Oct 14, 2019

wusyong Oct 14, 2019

SuperFluffy commented Oct 14, 2019 via email

wusyong Oct 14, 2019

wusyong Oct 14, 2019

samuel-rufi Oct 15, 2019

SuperFluffy commented Oct 15, 2019 •

edited

Loading


		# Summary

		This feature is responsible for the creation and interpretation of transactions and bundles.


		# Drawbacks

		Without any bee-model crate, nodes can not exchange transactions. Therefore this crate seems necessary.

	signature_fragments: String,
	signature_or_message: Option<String>,


		- Introducing a fluent API to the TransactionBuilder?
		- Should we use Option<T> for the fields in TransactionBuilder?

		\| `signature_or_message_fragment` \| contains the signature of the transfer \| \|
		\| \| or user-defined message data \| 6561 \|

		\| `address` \| receiver (output) if value > 0,
		\| \| or sender (input) if value < 0 \| 243 \|

RFC: Transaction type #3

RFC: Transaction type #3

Conversation

samuel-rufi commented Sep 6, 2019 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperFluffy left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunfardo314 commented Oct 2, 2019

Choose a reason for hiding this comment

SuperFluffy Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperFluffy commented Oct 14, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuperFluffy commented Oct 15, 2019 • edited Loading

samuel-rufi commented Sep 6, 2019 •

edited

Loading

SuperFluffy Oct 10, 2019 •

edited

Loading

SuperFluffy commented Oct 15, 2019 •

edited

Loading