ethereum / EIPs

The Ethereum Improvement Proposal repository
https://eips.ethereum.org/
Creative Commons Zero v1.0 Universal
12.88k stars 5.28k forks source link

web3.js contract object re-design #68

Closed frozeman closed 5 years ago

frozeman commented 8 years ago
ERC: 68
Title: web3.js contract object re-design
Status: Draft
Type: Informational
Created: 08.01.2016
Resolution: https://github.com/ethereum/web3.js

Abstract

This proposal is a move to make it a more future proof javascript representation of a ethereum smart contract.

Motivation

The contract object in web3.js in its current form has confused in many ways and has some draw backs, like not being able to pass a struct (object) as a functions arguments.

Usage

// contract options can be
options = {
    data: '0x12345678', // used when deploying contracts, so users don't need to pass in the data later
    from: '0x12345678..', // default from when sending tx
    gasPrice: '0xb', // default gasPrice when 
    gasLimit: '0xa' // default gasLimit when 
}

// initiating an un"addresses" contract object
var myContract = new web3.eth.contract(abi [, options])
myContract.address = '0x12345678...'; // add address later

// initiating with address
var myContract = new web3.eth.contract(abi, address [, options])

// -> deploying a contract
eventemitter = myContract.deploy({
    arguments: [param1, param2],
    data: '0x2345678' // when provided, this is used over the one from the options object
});
eventEmitter.on('error', function(error){ 
 });
eventEmitter.on('transactionHash', function(hash){ 
 });
eventEmitter.on('mined', function(address) { 
     new web3.eth.contract(abi, address);
     // or
    myContract.address = address;
 });
// also promise style works, will be fired when mined
eventEmitter.then(function(address){
}).catch(function(error){
})

// -> get the call data for a contract deployment
myContract.encodeABI({
    method: 'constructor',
    arguments: [param1, param2],
    data: '0x2345678'
});
> 0x23456780000000000000000000005345345

// -> events
myContract.on('someEvent' [, {filter: {myValue1: 23, myValue2: [3,5], ...}, fromBlock:..}], function(error, returnValues, log) {})

myContract.pastEvents(('someEvent' [, {filter: {myValue1: 23, myValue2: [3,5], ...}, fromBlock:.., toBlock: ...}], function(error, returnValues, log) {
   // called for every past event.
});

// if a `filter` option is provided, it means either value can be contained in the matching event (log)

// -> methods
// -> methods
myContract.myMethod(param1).estimate({from: '0x1234...'}, function(error, result){})

myContract.myMethod(param1).call({from: '0x1234...'}, function(error, result){})

myContract.myMethod(param1).send({from: '0x1234...'}, function(error, hash){})

var eventEmitter  = myContract.myMethod(param1).send({from: '0x1234...'})
eventEmitter.on('error', function(error){ 
 });
eventEmitter.on('transactionHash', function(hash){ 
 });
eventEmitter.on('mined', function(receipt) { 
 });
// also promise style works, will be fired when mined
eventEmitter.then(function(hash){
}).catch(function(error){
})

// -> get the abi of a method
myContract.encodeABI({
    method: 'myMethod',
    arguments: ['hello', 12345] 
}) // get only the data of the call '0x23456787654323456765432340000000000000000000000034'

Summary

The contract object would then look as follow:

new web3.eth.contract(abi, address);
> {
   address: '0x123456...',
   deploy: function(options){...},
   encodeABI: function(options){...},
   // events
   on: function(event, options, callback){...},
   pastEvents:  function(event, options, callback){...},
   // methods
   estimateGas: function(options){...},
   call: function(options){...},
   transact: function(options){...}
}

Community Request

Please give comments and reasons, why we should go in one way or another. Please lets keep this ERC short, We don't want to spend multiple weeks discussing this ;)

obscuren commented 8 years ago

The gist of changes (please correct me if I'm wrong):

  1. create is renamed to deploy
  2. getABI has been added as an exposed method of the contract
  3. on(event) instead of Event

The myContract.on('someEvent') can become a real headache if improperly implemented. I can certain see the rational behind it but I urge you to do some event checking when events are added. The old behaviour let the compiler/interpreter do sort of type checking e.g. when MyEvent wasn't declared it would throw an error and right now would just silently ignore it (if incorrectly implemented). My only suggestion would be that when new filters are created that you do some checking whether a filter actually exist and in the case where it's missing you throw an exception. I understand this might be obvious to some but it is easily forgotten. This should certainly not be viewed as a this-is-how-you-should-code kind of comment :-)

I welcome the getABI method :pray: as I've been wanting it on so many occasions (though mainly for debugging).

I don't quite understand the rational behind the deploy method. What's wrong with create?

frozeman commented 8 years ago
  1. create was called new since a while already. I think deploy is more clear, as your actually deploy a contract on chain
  2. This was also already in in the form of contract.myMethod.getData(param1, param2). But this structure makes it more clean and could be even used for events
  3. it was right now all attached to the contract object, which can lead to namespace clashes: contract.MyEvent(..). Also the .on('MyEvent') is commonly used and understood in the JS world.
  4. The new method calling structure allows for passing objects (structs) as well, as we before couldn't differ between the last options object and a parameter.

Additional ideas:

We can think of using a parameter object, instead of function(param1,param2), so we can add types to the parameters to work with solidities same-name functions.

The only drawback in this proposal right now is that you have no contract factory object anymore. This allowed you to instantiate many objects from this "factory class", but also lead to a lot of confusion. This can be replaced by simply making the ABI globally (or contextual) available, which then can be used to instantiate a new contract object.

chriseth commented 8 years ago

Nice proposal!

Some ideas:

Events given as string: I think it's fine, but should also allow for parameter types to disambiguate and throw if the event does not exist.

getABI should be called encode, encodeFunctionCall or abiEncode. We should also provide stand-alone encode and decode functions that do not have to be tied to a certain function and can also work without function name (we have to provide the types, though).

Inside getABI we should use arguments instead of parameters (parameter is the abstract thing, argument is the value you substitude the parameter with).

For event filters: Can we please make it clear in the syntax whether we AND or OR the arguments?

Do we still allow myContract.myMethod(2, function(){}) for constant functions?

I think it is a good idea to separate the actual function arguments and the metadata like gas and sender.

frozeman commented 8 years ago

Thanks for you comment.

I changed it to encodeABI and also to arguments.

The old myContrac.myMethod is not supported anymore to avoid namespace clashes, therefore we have now myContract.call().myMethod() (which let you call/transact to any function. We might throw an console.warn if somebody transacts to a constant func.).

For the last comment, i added "if a filter option is provided, it means either value can be contained in the matching event (log)"

Is that fine, or what do you mean.?

coder5876 commented 8 years ago

Looks nice! :smile:

So in this usage

eventEmitter.on('mined', function(address) { 
     new web3.eth.contract(abi, address);
     // or
    myContract.address = address;
 });

you only get the address once the transaction creating the contract has been mined. Sometimes it might be desirable to create a contract and immediately send a transaction to it. Since the address only depends on the nonce the address is known right after the transaction is sent off. It might be useful to have the API return the address after the transaction is sent. Would that create other issues?

frozeman commented 8 years ago

It is most likely the right address, if you predetermine it from the nonce, but there are cases where it could fail (e.g. a different tx got in before you etc, but then it might fail right away).

I will think about adding it.

gavofyork commented 8 years ago

The API doesn't seem to handle microforking (i.e. chain reversions for these events) - any plans for that? Or suggested workarounds?

frozeman commented 8 years ago

They will, the callback is fired with the log object, which will have the status: 'deleted' or something. This is more a node thing. Though i might add this as a fourth param to the called callback or so.

VoR0220 commented 8 years ago

I very much approve of the handling of structs. This will make SO many things easier when it comes to solidity.

tcoulter commented 8 years ago

I'd like to introduce ether-pudding (a.k.a., "Pudding") for those that aren't familiar. Pudding is a similar abstraction layer for Web3, but it goes beyond what Web3's abstraction layer does, and even this ERC. My end goal for Pudding was always for it to not exist, and so I hope that its features can eventually get consumed.

Regarding the ERC, I'm very excited about myContract.transact(...).on("mined"). That was one of the original impetuses for creating Pudding, and I'm happy that my code no longer needs to be there. That said, there's a lot more the abstraction layer can do to make life better for developers, and I've included some suggestions below, using Pudding as an example.

  1. Integrate Promises, or some mechanism for transaction and call chaining. In Pudding, an example coin contract including a mixture of transactions and calls might look like this:

    var self = this;
    this.displayLoadingIcon();
    metaCoin.sendCoin(receiver, amount, {from: fromAccount}).then(function(tx_hash) {
     // This callback will only be executed once transaction has been mined
     return metaCoin.getSendersBalance.call({from: fromAccount});
    }).then(function(balance) {
     self.removeLoadingIcon();
     self.displayBalance(web3.fromWei(balance, "ether"));
    }).catch(function(e) {
     alert("Oh no, there was an error!");
    });

    In this example, we send some "meta coin" to the receiver, then refresh the balance of the sender once the transaction is finished processing. Obviously, I've inserted other functions that might exist within this this application to communicate what's happening to the user; but it's very clear what the app is doing and in what order it's doing it. Note that this structure is very good for unit tests, in that you don't want to start on one step until the one before it has been completed.

  2. Provide a way to set default transaction values per contract, per contract instance, or perhaps globally. As an example, specifying the from address for every transaction is tedious, and is even worse if you need to specify the gas price and gas limit as well. With Pudding, this is easily solved in a few lines of code, where I can create an instance of the MetaCoin contract that applies to a specific address:

    var myMeta = MetaCoin.at("0x1234...");
    myMeta.defaults({
     from: "0xabcd...",
     gasPrice: "100000000000" // random number
     gasLimit: "100000" // also random
    });
    myMeta.sendCoin(200).then(function() {
     return myMeta.getSendersBalance.call();
    })...

    Perhaps even better, this can be integrated into the at or deploy functions:

    var myMeta = MetaCoin.at("0x1234", {from: "...", gasPrice: "...", gasLimit: "..."});

    In these examples, for each call or transaction the from address "0xabcd..." is used because it was specified as the default. The same is true for gasPrice and gasLimit.

  3. Create a way to serialize contracts, with ABIs, binaries and deployed addresses into a Javascript file that can then be included in your project via require() or import. Pudding can already do this for you, if asked, and tools like Truffle already take advantage. The benefit is you can use tools like webpack, browserify, etc. to easily include your contract and contract's assets within your project in a manageably way.
  4. Lastly, like Pudding, the contract's binary should be supplied in the initial object creation instead of passed during deploy(). Proposed example:

    var myContract = new web3.eth.contract(abi, binary);

    Or currently, in Pudding:

    Pudding.whisk({abi: ..., binary: ..., address: ...})

    There are negatives to passing the binary during deploy(). The first is that you could easily send data to .deploy() that doesn't correspond with the abi that was passed into web3.eth.contract() originally. Worse, however, is that you now have to make that binary accessible to any part of the code that needs to create a new instance of that contract. This requires a lot of parameter passing and is very tedious. Instead, it should only need to be specified once, and the contract object should manage that binary.


Given all the features I mentioned together, interacting with contracts becomes much easier, and in total there's less to think about. Here's a Pudding example:

// Read in the contract from a file that contains the abi, binary, and other metadata.
// .load() is an idiosyncrasy; we need to ensure the same Pudding instance is used throughout
var MyContract = require("./compiled-contracts/MyContract.sol.js");
MyContract.load(Pudding); 

// Set the defaults class-wide, and not just instance-wide.
MyContract.defaults({
  from: "0x1234..."
});

// Create a new version of the contract, without having to worry about the details
var myContract;
MyContract.new(function(instance) {
  // Instance is now on the blockchain, and can be interacted with
  myContract = instance;

  // Call someFunction() on the instance.
  return myContract.someFunction();
}).then(function(tx_hash) {
  // Now read some data, since that transaction has been mined.
  return myContract.readSomeData.call();
}).then(function(some_data) {
  // Do something with the data we just read from the network.
  ...
}).catch(function(e) {
  // Handle errors throughout the whole promise chain
});

I think we have a chance to make interacting with the blockchain less tedious for the developer. Pudding is a step forward, but I'd very much hope Pudding doesn't have to exist. This ERC still requires the developer to think about a lot of moving parts, like mining, or dealing with the hash, etc. I think in general we should focus on reducing the amount of things the developer has to think about, and work on making the resulting code clearer.

Aside: You can think about mining, transaction hashes, _not_ waiting for a transaction to be mined, etc. with Pudding, if you want to. But that's not Pudding's -- and I think our -- main goal.

VoR0220 commented 8 years ago

@tcoulter is spot on. Promised are an awesome addition and should be a standard for web3.

coder5876 commented 8 years ago

:+1: for @tcoulter's points! Pudding is very nice, I use it for all my web3 projects. Good that we have callback on mined transaction, Promises would be very nice in web3!

frozeman commented 8 years ago

@tcoulter i agree with all of your points, though with slight differences:

  1. Integrate Promises: I agree, but how would you "only" return the tx has, without waiting for it to be mined? This removes this low-level way of dealing with confirmations etc yourself. You also have no way for the user to show a "in progress" after you send the tx, as there is no callback fire immediately. I rather here go with the eventEmitter, as proposed in the ERC, as it allows for multiple events.
    Though this does not mean, that promises are good for all the other method calls we have. I'm actually all for removing the sync all together, as its a no-go anyway.
  2. "Provide a way to set default transaction data [in contracts]": Thats a very nice addition and i will add this to the proposal
  3. "Create a way to serialize contracts": awesome idea, but that might be better in a separate tool like yours
  4. "the contract's binary should be supplied in the initial object": Thats definitely something i will add as an optional option. But not as an required one, because sometimes you deal with contracts, you never want to deploy, nor do you need to know the binary. You simply want to interact with them.

Thanks again for all the great feedback. If anybody want to help me integrating these new features, please shot me a message, as i'm only one person with two hands and 3 projects ;)

frozeman commented 8 years ago

Follow the progress: https://github.com/ethereum/web3.js/blob/newContract/lib/web3/contract.js

I would appreciate any help.

VoR0220 commented 8 years ago

@frozeman The way I understand promises in pudding, it does just that in waiting for the transaction to be mined which then sends a callback, and then the promise clears and moves onto the next step.

Furthermore, why should serializing contracts be in a separate tool, just out of curiosity?

tcoulter commented 8 years ago

Great @frozeman, glad this helps!

Regarding this point:

I agree, but how would you "only" return the tx has, without waiting for it to be mined?

Pudding accomplishes this by having two different functions, one that waits to be mined and one that doesn't. In my experience, I use the function that waits to be mined 99% of the time. Here's an example:

var myContract = MyContract.at("0xabcd...");
myContract.someFunction(function(tx_hash) {
  // This callback won't be called until the transaction is mined.
  // Arguably, Pudding should also return the transaction receipt here, but it doesn't currently
});
myContract.someFunction.sendTransaction(function(tx_hash) {
  // This callback will be called immediately after the transaction has been submitted
});

The default behavior for Pudding is to wait until the transaction has been mined, however you can circumvent this by calling sendTransaction instead on each function. I think this is a good default while also allowing developers the flexibility to manage their transactions in a different way. Promises, in this case, are mostly for transaction and call chaining when waiting on a transaction to be mined and avoiding "callback hell" in the process. Note that with the .on("mined", ...) behavior in the ERC, you still get the equivalent of callback hell as you might initiate a second transaction or call within the on("mined", ...) callback, which would then result in another on("mined", ...) callback, and so on and so forth.

Regarding @VoR0220's question:

Furthermore, why should serializing contracts be in a separate tool, just out of curiosity?

I'm assuming that's because it adds a lot of dependencies to web3, and were it to perform all the serialization functionality web3 would have to write to the filesystem, which only works in some environements (i.e., Node). I'm fine with this, actually. Pudding would then become a code management tool more than an abstraction layer, which is a fine separation, I think.

Cheers!

frozeman commented 8 years ago

I agree that chaining for function calls etc is nice, and i'm all for promises. In fact i wrote a whole API layer on promise basis a few years back.

But in this case i don't think promises, or two separate functions is the right way.

The advantage of the event emitter is that it allows us to later add even more events, which can then even be subscriptions under the hood. I can think of events for confirmation, which fires every block for "x" blocks. And a confirmed event, which fires after the "x" block.

It allows to use web3.js low level, as well as convenient while having a clear API.

Promises can only do one action after a certain time, and are therefore only really useful when we do contract function and method calls.

tcoulter commented 8 years ago

Promises can only do one action after a certain time, and are therefore only really useful when we do contract function and method calls.

I agree with all your points, but in my experience "doing contract function and method calls" is 99% of my interaction with the Ethereum blockchain. We can still emit events for more advanced things, but I think we should focus the main use case on the features people will use most often.

Regarding promises specifically, I don't care if we use promises or some other chaining method, but I'd really like to avoid code like this, which is how most of my code would become:

myContract.transact(...).someFunction.on("mined", function() {
  myContract.transact(...).someOtherFunction.on("mined", function() {
    myContract.call(...).yetAnotherFunction(function() {
      // two transactions and a call, as a simple example
    });
  });
});

This seems less clear. We can have events and chaining (chaining without promises). It's possible, and I'd be happy to try and code up an example.

Smithgift commented 8 years ago

I'd be very happy with these changes, but I'm just concerned that they're changes. I'd have a large amount of code that would need rewritten if this API replaced the current API, and I'm sure other projects would have similar amounts of code to rewrite.

Can we work some under-the-hood JS magic and have web3.eth.contract return (deprecated) old-style objects if simply called and new-style contracts if new is used? (I have no idea if this is even possible.) Alternately, if that is impossible/would awaken foul horrors from the depths of time, could we get away with specifying the version somewhere?

I suppose, however, that inevitably that web3 has to make breaking changes, (Like, say, when web3.sha3 works as one expects it would.) For my own major project, I get web3 from browserify, so I could probably just specify the version using npm. The real question is: how will mist provide web3 in the future, and how much guarantee will developers have that web3 will remain stable?

frozeman commented 8 years ago

@tcoulter i understand you callback hell fear, but only returning promises, which are resolved when its mined limits you for when you need the tx has directly.. events can have multiple event types and therefore are more flexible.

How would you allow chaining while being flexible??

@Smithgift we will need to make breaking changes, and users can choose their own web3.js version. We are still a very small community and therefore i want to get these changes in ASP, to avoid this in the future to often.

Concerning Mist. Ideally the dapp developers always uses its own web3.js version. The mist is only there for the simple dapps

frozeman commented 8 years ago

@tcoulter do you think about some crazy promise style event like?


contract.transact().myMethod().on('mined').then(function(){   })
tcoulter commented 8 years ago

@frozeman I was more thinking something like this, which returns a promise-like object that's also an event emitter:

var transaction = contract.transact().myMethod();
transaction.on("transactionHash", function() {
  // This function will be called when the transaction is returned 
  // (i.e., once transaction is submitted)
});
transaction.on("mined", function() {
  // This function will be called when the transaction is mined
});
transaction.then(function(tx_hash) {
  // This is equivalent to transaction.on("mined"), and fires at the same time.
  // Here's where the chaining happens: returning a new transaction object for
  // the next transaction, like you can do with promises.
  return contract.transact().otherMethod();
}).then(function(tx_hash) {
  // Again, this won't be fired until the transaction is mined.
  // In addition, we should support calls in chainable format. This is arguably 
  // more important as I make more calls than transactions, one after the other:
  return contract.call().yetAnotherMethod();
}).then(function(return_value) {
  // Do something with the call's return value.
}).catch(function(e) {
  // It's important that we support catching errors across the whole chain, like promises.
});

Implementation-wise, I think there are two avenues to achieving this:

  1. Create our own class-like object which extends from EventEmitter, adding in the promise functionality ourselves.
  2. Use an already-created promise library (i.e., bluebird, or native Promises) and add in the EventEmitter behavior on the object we return from contract.transact().myMethod();.

I think both are doable. If we choose the first, we should be sure to use the same Promise interface so we can easily integrate with other promise libraries and get all the benefit of Promises without having to completely rewrite the whole library (for instance, I use Promise.all() a lot to fire off many transactions at once that all need to be mined before continuing). If we choose the second (my favorite) we can just return a custom promise that inherits from EventEmitter, giving us the best of both worlds. In this case, I suspect we'd choose native promises so we don't have to include another dependency in web3.

tcoulter commented 8 years ago

PS: I'm on the edge of my seat whether you end up liking this. So excited if this gets in. Also happy to help with implementation.

VoR0220 commented 8 years ago

:+1: @tcoulter 's proposal. I really, really think we should enable some kind of promise into the transaction sending. It makes coding and testing for these things soooooo much easier.

frozeman commented 8 years ago

Sounds like a good idea. I already started here: with the eventEmitter, we could simply add promise support then i guess. What promise library is best, or native?

I used Q from kritokwal a while back

frozeman commented 8 years ago

@tcoulter this was an awesome idea! The magic here is:

var eventifiedPromise = function() {
    var resolve, reject,
        emitter = new EventEmitter(),
        promise = new Promise(function() {
        resolve = arguments[0];
        reject = arguments[1];
    });

    // add eventEmitter to the promise
    promise.emit = emitter.emit;
    promise.on = emitter.on;
    promise.once = emitter.once;
    promise.listeners = emitter.listeners;
    promise.addListener = emitter.addListener;
    promise.removeListener = emitter.removeListener;
    promise.removeAllListeners = emitter.removeAllListeners;

    return {
        resolve: resolve,
        reject: reject,
        promise: promise
    };
};

I implemented and tested it with contract.deploy() and it seem to work fine. See https://github.com/ethereum/web3.js/commit/9878c8604dacbd781ab4560a0c0eb1c89ace2052.

This now allows deploy (and later other contract methods) to allow: callbacks, promises and events!!

So everybody can choose which one to use.

Example:

myContract.deploy({from: '0x12345...', data: '0x234567...', arguments: [123]}, function(e, res){ console.log('Callback results:', e,res)});

event.on('error', function(error){ console.log('Event error:', error)});
event.on('transactionHash', function(hash){ console.log('Event transactionHash:', hash)});
event.on('mined', function(address){ console.log('Event mined:', address)})

event.then(function(address){ console.log('Pomise mined:', address)  }).catch(function(e){ console.log('Promise error: ', e); });

> Promise {[[PromiseStatus]]: "pending", [[PromiseValue]]: undefined}

> Callback results: null 0xcea73979a424ebd2c1e4ecdd0844c5cb4644e92cf04f7e5f1cc5d87e69ee5168
> event transactionHash: 0xcea73979a424ebd2c1e4ecdd0844c5cb4644e92cf04f7e5f1cc5d87e69ee5168
> Event mined: 0xa1fb3680c055ba95f24bacb59ed1b32cbc699326
> Promise mined: 0xa1fb3680c055ba95f24bacb59ed1b32cbc699326

BTW i clear all event listeners automatically after an error or after the mined event, as no other events will ever be fired. You guys think thats a good idea?

frozeman commented 8 years ago

Made contract.encodeABI work:


contract.encodeABI({method: 'constructor', arguments: ['0x23456789'], data: '0x12345678912345678'})
> "0x123456789123456780000000000000000000000000000000000000000000000000000000023456789"

contract.encodeABI({method: 'seller', arguments: [2]})
> "0x37ab20b00000000000000000000000000000000000000000000000000000000000000002"
rfikki commented 8 years ago

This all sounds great. What kind of time frame are we looking at for this release?

tcoulter commented 8 years ago

Awesome @frozeman! Nice work! :clap: :moneybag:

BTW i clear all event listeners automatically after an error or after the mined event, as no other events will ever be fired. You guys think thats a good idea?

Sounds like the right plan. If there's an error you don't want things to continue.

So excited for this. Are you just using native promises? I'm assuming native is the best way to go to reduce dependencies, though my favorite library is bluebird. I'd only consider using a library if the execution environment didn't support native promises. Looks like browsers are cool with it. Need to find more data about Node.

frozeman commented 8 years ago

@rfikki the contract object might be done soon, but i have some more changes planned for web3.js and we move completely to pub/sub (PR already ready) So this is part of the overhaul for web3.js 1.0. If people help me we can get there faster, on the end its only me working on it right now (and i have also mist and the wallet as work)

Parts of this also depend on how fast we get changes into geth and other clients.

@tcoulter i used native promises, as you can see here: https://github.com/ethereum/web3.js/commit/9878c8604dacbd781ab4560a0c0eb1c89ace2052

Though i'm not sure if i should load bluebird as backup, e.g. for IE. Node.js works fine with promises, at least the version i have running. Native promises and blue bird seem to work exactly the same.

Smithgift commented 8 years ago

In, re: clearing all events, I would be a little cautious before clearing the "mined" events, just in case there's a microfork and the transaction is reverted. Clearing all on error makes sense.

frozeman commented 8 years ago

I see your point.

But even if there is a microfork, you wouldn't get the mined event fired again, as its a event internally in web3.js, which once fired is "done".

Though i can imagine moving the mined event at some point to the pub/sub as a subscription inside the node, then this might make sense.

I could also add checking and confirmation events later on.

The good thing about the current structure is that we can improve the under-the-hood behaviour, as well as add new event types

IstoraMandiri commented 8 years ago

Great work guys, I like pudding's style a lot and I'm glad chained promises will become standard.

Also +1 to @frozeman's suggestion:

I can think of events for confirmation, which fires every block for "x" blocks. And a confirmed event, which fires after the "x" block.

This will make things like progress bars really easy to work with. Would there also be a way to access this data without an event emitter, for longer confirmation times; perhaps something like an age property?

VoR0220 commented 8 years ago

@iurimatias this is something to absolutely keep an eye on for Embark.

frozeman commented 8 years ago

@hitchcott can you elaborate on your last idea a bit?

IstoraMandiri commented 8 years ago

Let's say there's a contract that needs 100 confirmation blocks and for whatever reason the event emitter is stopped (user closes window, navigates to different route). If the user navigates back to the contract view, we want to immediately show the number of confirmation blocks without waiting for a confirmation event, so it'd be nice to have some way of simply getting the number of blocks since mined (confirmations).

Smithgift commented 8 years ago

Suggestion for the future (I think this isn't possible with the current EVM): a "out-of-gas" event. It's not technically an error, but it's worth telling the developer. (Really, you're best off listening for actual contract events and acting on them, rather than assuming a transaction did what it was supposed to. But going out-of-gas is a significant event.)

frozeman commented 8 years ago

@hitchcott this would require a lot of calls under the hood, which i would like to keep out of web3.js for now. We shouldn't add every simple feature we see fit to web3.js (yet), rather take a smart approach and first implement this in our dapps. Once we see its a pattern we all repeat constantly then we can think of adding it to web3.js.

@Smithgift good idea to add the out of gas, event

barkthins commented 8 years ago

@frozeman @hitchcott Please make web3.js extensible so that users can add their own logic regarding transactions e.g. @hitchcott 's example. I have low confidence and high confidence code (i.e. I don't care if an Oracle update doesn't get mined, but I care a lot if ether didn't actually get sent) and we need to be able to differentiate the two. I'm fine writing this myself, but only if web3.js makes it easy to do without a pile of logic in my application code. For example, a wrapper function where I consume a blockConfidenceLevel parameter would work, but is that possible to easily do now?

Another area is sending maxGas is fraught with peril as currently an exception will consume USD $1 of gas. Yikes. I'd like to be able to monitor how close to gasSent an API call is getting but I definitely don't want that code in my application logic. If I can extend web3's notion of a transaction with my own logging magic I'm fine writing it myself... but at least make that possible. Right now the concept of which solidity API got called and how much gas was used are in completely separate areas of the code, which means I'm littering my application logic with that type of code. You could for example make it easy to attach the ABI signature to the transaction receipt, then it's a simple from the application side, and a separate concern for logging the different between gasSent and gasUsed.

A third example is most applications won't care all that much about the transaction hash the application logic. The application really should get an exception if a transaction isn't mined in a reasonable amount of time, and other than that application logic doesn't care much about the transaction results. In fact the only "returned" part of a completed transaction that application logic will care about is the txlogs, which currently aren't parsed. Make it easy for the user to extend the transaction code in web3.js to add parsing of the txlogs without that logic littering up the application code. @tcoulter posted a great example on stackexchange of how to do this. See how hard it would be for an end-user to add that code to web3.js or a wrapper on web3.js as a test case.

Again, I'm not asking you to code these features, I'm asking you to make it easy for the user to code these features without having to litter them into their application logic.

barkthins commented 8 years ago

Another extensibility comment: rxjs allows you to define which promise library to use. bluebird IMHO is more efficient and easier to debug than native. So allow the user to define what promise library to use.

from example straight out of my code:

var Promise = require('bluebird');
var Rx = require('rx');
Rx.config.Promise = Promise;
frozeman commented 8 years ago

@barkthins i agree with the flexibility argument.

Could you provide some example code of what it is you would like to have, its hard for me to understand your problem if you write a bunch of texts of what you like to have.

Also could you link @tcoulter stack overflow issue?

Also keep in mind, web3.js is a low level API library not an all in wonder solution for every dapp, so adding more logic will grow it internally and i try to keep the calls we do internally to the node at a minimum.

Additionally could you explain me you promise logic? I use bluebird as a fallback currently, but how would you configure the promise library from the outside? you want web3.js to expose the Rx object?

I'm want to keep web3.js lean and not add any additional new cool library..

barkthins commented 8 years ago

@frozeman let's start a conversation on gist, I have example code there. Let's try and modify it to meet the requirements I listed.

https://gist.github.com/barkthins/c2e527af82056542cd7d

barkthins commented 8 years ago

you want web3.js to expose the Rx object?

No, that was an example of how to specify a promise library for a module.

RXJS definitely should not be in web3.js. I hook RXJS off of the event callbacks and it works great, no need to put that into web3.js since it's easy for users to add as needed.

pipermerriam commented 8 years ago

Is this still happining? I was planning on updating my python-based tooling to conform to this API but I don't want to put in the effort unless this is going to be real.

graup commented 8 years ago

Oh, how I would love to have proper promises. Making transactions and events chainable would improve my code a lot.

Would that also allow us to add something like Contract.once('MyEvent', (error, result) => { doSomethingOnce(); });?
I find that very often I need to wait for exaclty one event, but currently I have to filter.stopWatching(); in the callback.

Schaeff commented 8 years ago

Having promises as a default tool for web3 would be really awesome. Working with Pudding has been a bliss, and adding the flexibility of events looks great.

Any update on this?

aakilfernandes commented 8 years ago

@Schaeff There's this https://github.com/SafeMarket/web3-q but it wraps the default web3 functions rather than overwriting them for upgrade reasons.

nicksavers commented 6 years ago

@frozeman Can be closed? Or do you prefer to wait until https://github.com/ethereum/web3.js/pull/558 (web3.js 1.0) is merged?

nicksavers commented 5 years ago

Closing this as the web3.js 1.0 branch has become the default.