Bitcoin Forum
April 24, 2019, 12:56:43 PM *
News: Latest Bitcoin Core release: 0.17.1 [Torrent]
 
   Home   Help Search Login Register More  
Pages: « 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 [43] 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 »
  Print  
Author Topic: 300 BTC Coding Contest: Distributed Exchange (MasterCoin Developer Thread)  (Read 128813 times)
This is a self-moderated topic. If you do not want to be moderated by the person who started this topic, create a new topic.
Tachikoma
Hero Member
*****
Offline Offline

Activity: 938
Merit: 1000



View Profile WWW
January 13, 2014, 10:43:39 AM
 #841

Devs: Could you +1/-1 this pull so we can quickly merge it: https://github.com/mastercoin-MSC/spec/pull/35#issuecomment-32158885

Electrum: the convenience of a web wallet, without the risks | Bytesized Seedboxes BTC/LTC supported
1556110603
Hero Member
*
Offline Offline

Posts: 1556110603

View Profile Personal Message (Offline)

Ignore
1556110603
Reply with quote  #2

1556110603
Report to moderator
Advertised sites are not endorsed by the Bitcoin Forum. They may be unsafe, untrustworthy, or illegal in your jurisdiction. Advertise here.
1556110603
Hero Member
*
Offline Offline

Posts: 1556110603

View Profile Personal Message (Offline)

Ignore
1556110603
Reply with quote  #2

1556110603
Report to moderator
Tachikoma
Hero Member
*****
Offline Offline

Activity: 938
Merit: 1000



View Profile WWW
January 13, 2014, 11:04:44 AM
 #842

No that's MST, this is MSC.

Electrum: the convenience of a web wallet, without the risks | Bytesized Seedboxes BTC/LTC supported
grazcoin
Sr. Member
****
Offline Offline

Activity: 284
Merit: 250



View Profile
January 13, 2014, 02:22:25 PM
 #843

Devs: Could you +1/-1 this pull so we can quickly merge it: https://github.com/mastercoin-MSC/spec/pull/35#issuecomment-32158885

just commented.

fthoughts
Newbie
*
Offline Offline

Activity: 21
Merit: 0


View Profile
January 13, 2014, 08:47:50 PM
 #844

I've commented as well.
zathras
Sr. Member
****
Offline Offline

Activity: 266
Merit: 250



View Profile
January 14, 2014, 04:46:12 AM
Last edit: January 14, 2014, 09:10:23 AM by zathras
 #845

Simplifying the Class A Model

Sorry this has taken a few days to get my ducks in a row (will be great when I go RBB!), but I wanted to make sure what I'm proposing tests successfully and will give us the outcomes I'm looking for (simplify Class A & remove ambiguity on whether a transaction is valid or not).

I've been discussing with Tachikoma for a few days, and I think I've now done enough testing to make this official & submit a pull.

What?
This pull is structured to simplify the Class A model by removing a number of potentially confusing rules and replacing them with a single defining rule.

Why?
Class A is only intended to provide backwards compatibility & is not used in any of the respective wallet implementations or for any features other than basic simple send.  Thus the effort being expended on Class A does not correlate with its utility to the project and this pull is intended to put Class A to bed so we can move onto other more useful work.

How?
All of the following rules would be removed from the spec:
Quote
The following conditions must also be satisfied for the transaction to be considered decode-able:

* The reference address sequence number must be the data address sequence number + 1
* Ideally, all outputs should be the same (except the change). In fringe cases where the change output value is equal to the other output values the change address can be identified by sequence number, which must not equal or be +/-1 of the sequence number for either the reference address or the data address
* A last resort 'peek and decode' method may be used to identify the data packet in the event of ambiguity following the above rules. This involves decoding each packet and looking for the correct bytes for a simple send (the majority of bytes in a Class A simple send do not change). These byte checks are defined as:
     + Bytes two to eight must equal 00
     + Byte nine must equal 01 or 02
* Should there still be packet ambiguity or 'peek and decode' reveals more than one packet (simple sends are always one packet) the transaction is considered invalid.
And replaced with:
Quote
* Has exactly two non-exodus outputs (one of which must be the data address) with a value equal to the Exodus output and/or has exactly one output with a sequence number +1 of the data address for reference output identification
EDIT: Qualified that one of the two same valued outputs must be the data address & that the two same valued outputs are in addition to the Exodus output.

A transaction will either satisfy this single rule or not, helping to remove some of the discussion/ambiguity around whether certain transactions that are decodable (eg via P&D) but don't meet one or some of the original rules should be validated.  The rule has been written to cater for all existing transactions and new code based on this simplified logic has undergone a fair degree of testing.

This is not a change to the underlying structure of Class A and more a consolidation of wording/logic such that we have a clear interpretation of what is and what isn't a valid Class A transaction.  This also does not enforce a method for testing against the rule - the rule is intended to be clear enough that no specific approach needs to be defined.

I have already rewritten the Masterchest library to utilize this logic instead for the purposes of testing (source).  This has resulted in a big reduction & simplification in Class A parsing code.  I've run the modified library against a full blockscan in the engine, and can confirm that all transactions validated by the original complex rule implementation have been also been validated by the simplified model.  Additionally transactions that were under active discussion between Tachikoma & I which were decodable via P&D but technically invalid per spec when considered against the original ruleset are now validated via the new single rule.

While trying to break this simplification I was able to identify an edge case of an incorrectly structured transaction that would cause ambiguity through this rule.  Essentially same output values provide one reference address while dataseqnum+1 provides a different reference address.  This edge case exists just the same in the original ruleset and has never been used in a transaction (that I could locate, happy to be corrected) - an example of such an incorrectly built transaction is as follows:

Code:
SEQNUM      ADDRESS             OUTPUT VALUE
101         ChangeAddress       0.05 BTC
100         DataAddress         0.00006 BTC
72          ReferenceAddress    0.00006 BTC
148         ExodusAddress       0.00006 BTC

As you can see the seqnum for the reference address should be 101 following dataseqnum+1, but that's actually the seqnum of what would be the change address following same output values.  For this specific case (where same outputs and dataseqnum+1 provide different ref addresses) in order to allow it we would need to set an order of precedence on whether same outputs or dataseqnum+1 is authoritative first and once we start down the road of which rule is more important than another we head back down into complexity territory.  As I say this is not a 'new' edge case specific to the simplification, but out of all the edge cases I looked at I think it is the one case that this simplification does not solve.  This is the very definition of ambiguity on the reference address so if it ever does appear on the network should be considered invalid IMO.

I have added a line in the pull to be explicit about this if we find an edge case is broadcast that results in ambiguity - "NOTE: Should a transaction result in an edge case that appears to follow the rules above but fails to exclusively and without ambiguity identify the sending, reference and data addresses the transaction is considered invalid."

TL:DR; Since this simplification caters not only for properly formatted transactions but also for every single edge case that has ever been broadcast for a Mastercoin transaction with just one single sentence - my own view is we should merge & put Class A to bed now and focus our efforts elsewhere.

To help with considering this pull, I have hooked up dev to prod to allow the consensus check to run against dev.  You can see the results of the simplified logic here (https://masterchest.info/consensus_masterchestinfodev.aspx).  Spoiler alert...



Shame about Exodus, I thought we had that locked up during the hackathon?  Other than that though, I recall offering a round of beers when we hit consensus - given our locations though you're all going to just have to buy yourself a beer and bill me for it Tongue

EDIT: Hmmm, how bizarre.  One of the changes we made during the hackathon (seconds in a year from 31557600 to 31556926) is MIA.  Put it back in, we're back synced on Exodus Smiley though Bitoy's is still a bit off.  I'd like to hear about update frequency from Grazcoin & Bitoy too if you can guys as I've noticed consensus seems to slip in and out depending on how recently the implementations update.  Consensus runs every 5 mins FYI.

Pull request is here https://github.com/mastercoin-MSC/spec/pull/36, please -1/+1 and feel free to comment/ask any questions.

Thanks Smiley
Zathras

P.S. Tachikoma, quick FYI took some hunting to isolate but squished the bug that was causing a second row for an address (1LjT88X7Zu8BdbqJw8vfRa83NJuzYL9kqm) in the balances table.

Smart Property & Distributed Exchange: Master Protocol for Bitcoin
Tachikoma
Hero Member
*****
Offline Offline

Activity: 938
Merit: 1000



View Profile WWW
January 14, 2014, 09:51:49 AM
 #846

Zathras let me start with thanking you for the effort you put in the above post, our approaches are totally different but I think your is perhaps the better solution as it leaves the actual implementation up to the implementor.

Your example of the transaction that breaks all these rules is actually the one I was talking about as well. I only assumed that because the transaction is always handwritten the Exodus sized outputs get preference over the non-Exodus valued outputs because it stands to reason the value that is not hand-crafted is not meant to be parsed for the transaction. If that makes sense.

Although since this transaction shouldn't happen anyway I think it would be safe to invalidate it. Could we somehow add a rule to the explicitly say which transactions to invalidate just because they are edge cases? Or should everybody understand that since it's not covered by the explanation?


How would you guys feel about organising a Simple Send test phase where we encourage users to send weird transactions over the network in order to break consensus. Could we be ready for that now?

Electrum: the convenience of a web wallet, without the risks | Bytesized Seedboxes BTC/LTC supported
zathras
Sr. Member
****
Offline Offline

Activity: 266
Merit: 250



View Profile
January 14, 2014, 10:19:34 AM
 #847

Zathras let me start with thanking you for the effort you put in the above post, our approaches are totally different but I think your is perhaps the better solution as it leaves the actual implementation up to the implementor.
Thanks for the patience mate haha - it's such an exciting project to work on but finding enough time is a nightmare atm as I'm sure it is for you Smiley

Your example of the transaction that breaks all these rules is actually the one I was talking about as well. I only assumed that because the transaction is always handwritten the Exodus sized outputs get preference over the non-Exodus valued outputs because it stands to reason the value that is not hand-crafted is not meant to be parsed for the transaction. If that makes sense.
That's actually a really good point and I'm sad to say I didn't consider that.  I have a tendency to come at things from a purely engineering kind of view and sometimes forget the human aspect of it.  Yep, since seqnum is out of the hands of the user and it's only the outputs they can control, I can see how there could be a case for giving same outputs priority over dataseqnum+1 in rare cases of conflict.  

How would you guys feel about organising a Simple Send test phase where we encourage users to send weird transactions over the network in order to break consensus. Could we be ready for that now?
I think it would be great.  There is a whole bunch of BTC set aside for testing and at the moment other than us devs I'm not sure how much other testing is going on - would be good to get some outside involvement, there's obviously plenty of reward so I don't think we'd have any problem getting volunteers.  We'd just seed some test MSC out and tell them to have at it Smiley

Smart Property & Distributed Exchange: Master Protocol for Bitcoin
Tachikoma
Hero Member
*****
Offline Offline

Activity: 938
Merit: 1000



View Profile WWW
January 14, 2014, 10:24:09 AM
 #848

I would actually suggest this should be done with real MSC (just really small amounts 0.00000001 etc.) because consensus on Test coins is already bonkers because of DEx Smiley

Electrum: the convenience of a web wallet, without the risks | Bytesized Seedboxes BTC/LTC supported
zathras
Sr. Member
****
Offline Offline

Activity: 266
Merit: 250



View Profile
January 14, 2014, 10:30:37 AM
 #849

I would actually suggest this should be done with real MSC (just really small amounts 0.00000001 etc.) because consensus on Test coins is already bonkers because of DEx Smiley
Fair point - let's see what Bitoy says about update frequency so participants don't think they've broken consensus just due to delayed updates - IIRC he's at 30 mins currently.

Smart Property & Distributed Exchange: Master Protocol for Bitcoin
Bitoy
Sr. Member
****
Offline Offline

Activity: 449
Merit: 250


View Profile
January 14, 2014, 01:37:42 PM
 #850

I would actually suggest this should be done with real MSC (just really small amounts 0.00000001 etc.) because consensus on Test coins is already bonkers because of DEx Smiley
Fair point - let's see what Bitoy says about update frequency so participants don't think they've broken consensus just due to delayed updates - IIRC he's at 30 mins currently.


I'm now updating every time there is a  consensus check.  (Hoping blockchain.org doesn't block my ip Smiley

Trying to update transactions for PDex checking.   I'll be making some changes to the database to meet tachikoma's dex API.

Finally 100% consensus Smiley
https://masterchest.info/consensus_masterchestinfodev.aspx
ripper234
Legendary
*
Offline Offline

Activity: 1274
Merit: 1000


Ron Gross


View Profile WWW
January 14, 2014, 02:17:48 PM
 #851

I would actually suggest this should be done with real MSC (just really small amounts 0.00000001 etc.) because consensus on Test coins is already bonkers because of DEx Smiley
Fair point - let's see what Bitoy says about update frequency so participants don't think they've broken consensus just due to delayed updates - IIRC he's at 30 mins currently.


I'm now updating every time there is a  consensus check.  (Hoping blockchain.org doesn't block my ip Smiley

Trying to update transactions for PDex checking.   I'll be making some changes to the database to meet tachikoma's dex API.

Finally 100% consensus Smiley
https://masterchest.info/consensus_masterchestinfodev.aspx

Terrific, this is a great day for The Master Protocol!
Thanks everyone for your tremendous efforts on this.


* I notice the Exodus address still has minor differences between the implementations, but I there's a pull request set out to fix that.

Please do not pm me, use ron@bitcoin.org.il instead
Mastercoin Executive Director
Co-founder of the Israeli Bitcoin Association
prophetx
Legendary
*
Offline Offline

Activity: 1498
Merit: 1001


he who has the gold makes the rules


View Profile WWW
January 14, 2014, 02:23:31 PM
 #852

Congrats!

I'm going to pop a bottle for you all

Bitoy
Sr. Member
****
Offline Offline

Activity: 449
Merit: 250


View Profile
January 14, 2014, 04:03:17 PM
 #853

To test  dex, lets just synchronize a few test msc addresses (maybe 4 addressess).

(We don't have to synchronize the whole test msc.)





udecker
Full Member
***
Offline Offline

Activity: 145
Merit: 252


View Profile
January 14, 2014, 05:51:54 PM
 #854

Guys, this really is awesome.  I do find it interesting that all four implementations have different exodus balances ;-)  Is it really only a single tx that causes the discrepancy?

JR, Peter and I have been having a side conversation about Mastercoin basic-SPV, btw.  There may be a way to ensure that all MSC transactions (from any client) are relying on the same set of information when they present a new transaction, in an easily verifiable method.  Not 100% sure at this point, but ideas are being discussed.  If this is the case, it might be easier in the future to ensure consensus on a go-forward basis.

Any concerns about all the Test MSC being spread all over the place, wrt DEx?  I like Bitoy’s idea of just syncing a few test MSC addys for consensus, and then figuring out a block where all DEx implementations should match after that point.

]
prophetx
Legendary
*
Offline Offline

Activity: 1498
Merit: 1001


he who has the gold makes the rules


View Profile WWW
January 14, 2014, 06:27:36 PM
 #855

Guys, this really is awesome.  I do find it interesting that all four implementations have different exodus balances ;-)  Is it really only a single tx that causes the discrepancy?

JR, Peter and I have been having a side conversation about Mastercoin basic-SPV, btw.  There may be a way to ensure that all MSC transactions (from any client) are relying on the same set of information when they present a new transaction, in an easily verifiable method.  Not 100% sure at this point, but ideas are being discussed.  If this is the case, it might be easier in the future to ensure consensus on a go-forward basis.

Any concerns about all the Test MSC being spread all over the place, wrt DEx?  I like Bitoy’s idea of just syncing a few test MSC addys for consensus, and then figuring out a block where all DEx implementations should match after that point.

the discrepancy is in the calculation of dev MSC vesting period caused by an issue with calculation based on time stamp, not any "transaction"
udecker
Full Member
***
Offline Offline

Activity: 145
Merit: 252


View Profile
January 14, 2014, 06:31:55 PM
 #856

the discrepancy is in the calculation of dev MSC vesting period caused by an issue with calculation based on time stamp, not any "transaction"

Ah - that makes more sense.  And since it’s Dev MSC, the devs have a very good incentive to make sure it’s correct!  Wink

]
ripper234
Legendary
*
Offline Offline

Activity: 1274
Merit: 1000


Ron Gross


View Profile WWW
January 15, 2014, 10:52:18 AM
 #857

I see a sudden drop in consensus, with a lot of conflicting addresses.
Also it doesn't seem like any one implementation is the cause for the conflict.

Copying pasting the conflicting addresses here (sorry for the format):

Quote
Consensus   Address   MyMastercoins   Masterchain   Mastercoin-Explorer   Masterchest
13ZDbTUK9prE7jVsjBz6t39JTihhifckHs   1   4.0   4.0   4.00
16D7dga3G2G1wAyFatF9HbCbCVkdRPmwUv      20.0   20.0   20.00
17KB39BPWiAEuY2g2cVibBQBWSaqTivdTr   20   0.0   0.0   0.00
181k55nxGuqDRpJx3iU43T4wJznf7ZazFN   0.043   0.043   0.043   
182osbPxCo88oaSX4ReJwUr9uAcchmJVaL   376.18   376.18   376.18   376.266
18ArFG8cPDT5P8NVdjFuSGjkybRTW8VBji   0.043   0.043   0.043   
1ATjSfdYVCBnFRrn383xHU7HvHt2EDWPni   1.01   4.01   4.01   4.01
1BSzMPgMwWXr8iQnJkU7Yaef7LFv4WaymY      1.0   1.0   1.00
1EXoDusjGwvnjZUyKkxZ4UHEf77z6A5S4P   11338.12465214   11347.28782561   11347.55421245   11347.30903753
1FBkQ5jbNkzuEVEU9mgTNrqSx73CBYhrbA      3.0   3.0   3.00
1GaNupdUBzfVF2B3JUAY1rZwHoXJgjyzXj   93.22275429   74.17245429   74.17245429   74.17245429
1KAsYCU9bs56UJWYsvWWfmCZmNgKG9Xis8      10.0503   10.0503   10.0503
1LjT88X7Zu8BdbqJw8vfRa83NJuzYL9kqm   60.12630845   60.12630845   60.12630845   10.00
1LxDxsJgS123R9XZntp6vafLJ2SoGfApFJ   165.16223427   211.73923424   211.73923424   211.73923424
1MaStErt4XsYHPwfrN9TpgdURLhHTdMenH   10450.27445588   10402.19745591   10402.19745591   10402.19745591
1Nymhex8hmirHLPMLHMVi9kmzdyjrXHcdg   2.227   2.727   2.727   2.727

Please do not pm me, use ron@bitcoin.org.il instead
Mastercoin Executive Director
Co-founder of the Israeli Bitcoin Association
ripper234
Legendary
*
Offline Offline

Activity: 1274
Merit: 1000


Ron Gross


View Profile WWW
January 15, 2014, 11:42:39 AM
 #858

Ron,

Clearly this is no big deal.  Probably MyMastercoins rolled back to an old version accidentally.  (It appears to me that only 'MyMastercoins' is off.  - no?)  I am sure they will set it straight on a simple tweek.  

You'll please note that the market price of Mastercoin rose 60% on the same day consensus was announced (yesterday/today).  Stupid - however, nevertheless true.  Now is a pretty bad time to shout out about apparent consensus failures.  Especially those which are clearly explainable and repairable.  Sometimes such notices aren't interpreted well by market players who are less than competent with regard to the meaning.  Mastercoin markets are like a theatre of people sitting on the edge of their seat.  One guy quietly utters 'fire' and the whole place is cleared in ten seconds.  Please, don't raise unnecessary alarms which are difficult for the non-techies to interpret with accuracy.

Thanks for the message.

I wasn't alarmed by this, just wanted to let the devs know that something is going on.
Mastercoin is a transparent project, if something breaks we let people know and fix it.

In this case it's important to let not just developers but also users know as they rely on this for transactions.

So ... not yelling "Fire!", just letting people know, and confident this will get fixed soon.

Please do not pm me, use ron@bitcoin.org.il instead
Mastercoin Executive Director
Co-founder of the Israeli Bitcoin Association
ripper234
Legendary
*
Offline Offline

Activity: 1274
Merit: 1000


Ron Gross


View Profile WWW
January 15, 2014, 12:01:22 PM
 #859

I wasn't alarmed by this, just wanted...
I do not suggest that you are alarmed.  I am merely commenting that your manner of notice - is the kind which cause market players to act like idiots. 

Your notice did not reflect the true nature of the problem.  Thus, these guys will go crazy.  It is certainly OK for users to know when the system goes out of sync - but I suggest one be careful with announcement which can be read recklessly. 

+1, I'll take that into consideration.

Please do not pm me, use ron@bitcoin.org.il instead
Mastercoin Executive Director
Co-founder of the Israeli Bitcoin Association
Bitoy
Sr. Member
****
Offline Offline

Activity: 449
Merit: 250


View Profile
January 15, 2014, 01:52:56 PM
 #860

I see a sudden drop in consensus, with a lot of conflicting addresses.
Also it doesn't seem like any one implementation is the cause for the conflict.

Copying pasting the conflicting addresses here (sorry for the format):


Hi Ron,

The mymastercoins engine is off for a few minutes.  Have to update the table to add tachikoma's dex api.

Consensus is back to 99.96% Smiley


Consensus   Address   MyMastercoins   Masterchain   Mastercoin-Explorer   Masterchest
1EXoDusjGwvnjZUyKkxZ4UHEf77z6A5S4P   11355.93262989   11355.93262989   11355.93262989   11355.93262988


There is only a 0.00000001 difference in masterchest exodus address.
Pages: « 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 [43] 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 »
  Print  
 
Jump to:  

Sponsored by , a Bitcoin-accepting VPN.
Powered by MySQL Powered by PHP Powered by SMF 1.1.19 | SMF © 2006-2009, Simple Machines Valid XHTML 1.0! Valid CSS!