 Velkommen alle til denne sesjonen, som heter Live Hacking Smart Contracts. Vi er ikke faktisk gammel til å gjøre detv-kon-stil-hacking i live systemet. Vi håper å ha en interaktiv sesjon med audiences, hvor vi diskusser sekuritetet. Vi diskusser det med en høy løvelsperspektiv. Nå er vi også tatt til å finne bug i det integrerende detaljene av kontrakten. Det blir tekniske glitser. Vi skal ha å switche over denne viewporten. Jeg håper du har noen patien med det. Jeg er Martin Hålsvendø. Jeg arbeider for The Ethereum Foundation. Det er etterisk sekuritet, og jeg har også vært med. Vi har Matthew Deferanti her, founder av CK Labs. Smart Contracts og auditing. Ja, kryptoresurter og sånt. Vi har Richard Moore. Åter av ethers.js. Og også randomstuff. Vi var jo sammen med The Awesome Nick Johnson. Vi kom ut av en E&S EMA. Så det ideet er at vi tar en del av kodet, som noen her sker og diskusser. Hvis vi kan, er vi veldig glade for at noen kommer opp på stedet. Vi har chatet med oss om kontrakten. Nå har vi fått en summasjon over Twitter. Så er Mr. Appleton present, og vil han joine oss? Jeg tror de ikke er her. Det er en liten andre. Det er godt å gjøre det. Det er en generell høy løvel. Hva er kontrakten? Vi har en bounce om hva som kan være lagt, og hva som kan være i stedet. Det er en ideer om hva vi ser ut for. Hvis dette kontrakten var forstået i dag, jeg tror alle de som var på denne stedet, er det noen av de som har sett noe interessant om å gå fra toppen tilbake, eller å si hva vi tror om dette. Det er en tokenkontrakte, som i de fleste oppgifter er det ganske normalt. Det har en owner. Det er en transfer. Det er en pattern her som jeg vil gi som en god eksempel, som jeg synes er en double dispatch ownership. Nei, det har ikke det. Det har en single dispatch. Når du transferer ownership til noe andre, har det en sjek til å se at du ikke transferer det til en annen owner. Og så setter det til den nye owner. En annen måte å gjøre dette, en double dispatch, er at du ikke setter en owner til en nye owner, du setter en potensiell nye owner til en nye owner. Og så tar det en nye owner til å gjøre et kvall til å acceptere ownership. Og det betyr at du vet for en fakt at denne owner av kontrakten er oppgjort av å gjøre det. Det er de som du har messet og interaktes med kontrakten. Nå har vi de ERC20 implementasjoner. Og vi har Safemath. Så... Nå er det ingenting vi kan se om Safemath. Det er en kvall standard. Altså, det kan være vort å vise når du ser på en annen kontrakte, du må kjenne å se om Safemath er oppgjort og de ikke er høyere i det. Du kan bare klasse over, og jeg tror det. Det er en ting som er gøy å ha kompilerne løp for å import kvallet som har dette hus. Fordi det er vort å være gøy å assume... Det er bra, det ser ut som det er Renz Eppelin. Eller E&S-name så du kan import Safemath.eth for at du kan være gøyere med hus. Du kan ikke. Hvis togene græver noe baser på hash. Hvis jeg kører på eterskan, jeg ser importer på eterskan. Jeg tror at det er safere mat og ikke noe sånn. Det er safer mat.eth, jeg har mer for eterskan. Så jeg tror at ethpm er tatt til å gjøre dette. Og med kompiler og integrasjoner, det er en god forhold. Her er CERC 20 implementasjonen av spesifikasjon. Så jeg vil sige til dere. Og til å give en kontekst, CERC 20 spesifikasjoner stateser at transfermetet skal rettes i Boolean, hvis transferet er succesfull eller ikke. Og som du kan se, og det er gammel å se, at det faktisk aldrig retter falskt. Det retter faktisk eller drar. Så hva var din oppindelig? Skal du gjøre en del av dette, og hvilken vil du rekommendere? Jeg vil definitivt gjøre en del av dette. Et annet kontrakt må være å gjøre dette. Og nå har du kallet det kontrakten til å fail. Som resultat av at det skal rettes falskt hvis det ikke fungerer, og at det skal fail ut av universitetet. Jeg er av oppfattelsen. CERC 20 spesifikasjoner at det skal rettes falskt og ikke rettes stateser, hvis du tryger å gjøre noe illegalt. Og jeg har tatt til å få folk til å rettes når noe illegalt skjønner er gjørt. For det er rettig å gjøre noe illegalt å trygge å gjøre noe illegalt med en andu mekanismer selv, vennom å relere med VMs mekanismer. Og hvis det er hårdt å gjøre en revert i soliditet, så er det en problem med soliditetet vi skal fixe. Jeg tror det er også bare... Det er kanskje bättre for at vi kan bruke denne år eller to til å bare bruke denne throwpattern, bare fordi jeg tror ikke at du ikke har gass for non-tribial kontrakter til å trygge å gjøre noe legalt. Det er altså en nødtemær til å trygge å gjøre noe legalt i normalt software. Hvis du trygger å gjøre noe legalt av VMs mekanismer, og sånt som soliditetet ikke holder ut av legalt, så jeg tror det er noe som som g Prepare der. Jeg tror, jo, så er det det Gran C20 standard. Det er litt dertig, fordi det kan bare rette til da, viss. Måste kontrakter gjør det, så hvorfor har du ikke til det? Det originale ideet var at det ikke skulle gjøre noe legalt. Det kjester ikke i forslag om du trygger å kjære mer tokens dan du har, og jeg tror det er brukt. Det er jo en overtremen, Specifikasjonen kind of sucks. We should still follow the specification and make an ERC 20 plus something that says don't do dumb things. Oh, so should it be fair, I don't think this is non-compliant because the ERC 20 doesn't say anything about whether you can throw or not. If you return successfully, you have to return true, but throwing in a case of a failure isn't prohibited. That's fair. And then like, this is like a real, or has been a real bad problem in the compiler where it's not strict enough, like you could, for example, write at least up until very recently, you could write this function without ever returning true, right? So like, take away that return true, and it would still like, it would just put a zero on the stack basically, right? Silently return false. And then between compiler version 4.22 and 4.23, like a bug was fixed with the call data return, where I guess like it was made more strict, so now those contracts actually return zero on the require. Yeah, so the output area is cleared before the call is made, and earlier the output area was the input already resided there. And because the compiler was too lenient now, like, if you compile a checking contract that requires, reps are required around the token, transfer, and the transfer goes through OK, but it returns zero now, like it properly allocates that zero, then the contract will throw, even though it's actually gone through correctly. I don't quite get that, I'll bug you afterwards. Another thing worth mentioning here is the lines 1.23 and 1.24, I think, I can't quite make out the alignment. 1.24 and 1.25 are redundant, if the, sorry, 1.24 isn't the check that the value is less than the balances is also enforced by SafeMath. Yeah, but it depends on the version of SafeMath you're using. OK, yeah. For the room to hear it, so the comment was that SafeMath fades on revert. Assert. Assert, right, and this version. And it's a good question, like it's kind of a philosophy question. Are you using SafeMath because you believe it should like throw in the case of an honest mistake, like the idea that if a SafeMath method ever fails, it represents an error in the contract, or are you doing it because you want it to automatically catch overflows for you? And one philosophy leads to reverse and the others will assert. So the comment from the audience is that we remove the SafeMath check on line 128 and instead do a regular minus. Yeah, so these are basically the same check. Well, the 1 on 129 checks against overflow, so 128 could succeed. So, and one important stuff to see that they got right here in this section is that you do the subtraction and addition in the right order, so that you don't like, I want to transfer 10 tokens from myself to myself and then you add it to yourself and then you can do it wrong so you get the double amount. If you do it the other way around and you have zero tokens, you can still transfer a million tokens to yourself. All right, let's move on. If there's any submissions coming in while we're talking, yeah, keep them coming. So it's not necessarily a security issue, but it is a Vince issue. If you add to the balance before you subtract and you start off with a zero balance and you're transferring to yourself, then you add a million tokens to your balance and then you subtract a million tokens from your balance and that succeeds. And it emits an event saying you sent yourself a million tokens, which shouldn't be possible. But wasn't there also a case of one of these tokens where you could increase your own balance? So the bad consequence would be that it could be a little fake overflow if you first increase the balance and if you transfer it to yourself, then you effectively double your balance and this can overflow even though this is impossible because the total number of tokens is less than the maximum amount. But I wanted to comment about the Safe Math implementation. Actually, it has some bad style there. I mean, this one implementation on top because it allows overflow actually to happen and then try to detect it afterwards. No, not this, but the implementation of Safe Math, right? Because it heavily relies on the particular behavior of overflow. Which particular one are you talking about? For example, this one, multiple multiplication, right? So it first allows overflow to happen at line 58, right? And then tries to detect it afterwards, heavily relying on what exact result. So it's obvious that if overflow didn't happen at line 59, everything will be fine. But the opposite is not obvious. I mean, it's hard to prove that if overflow actually happened, then condition at 59 will be violated. It's not that easy to understand and you need to know the particular behavior of how overflow works. I mean, what exact result will overflow return, right? So it's better to check before doing actual multiplication to check that it's easy check. I mean, you need to define max allowed value by one argument and compare to another. Yeah. And the same problem I think with addition. But not with subtractive subtractions, okay, but. Is this like from a practical point of view? No, actually the overflow behavior is predictable at least in EVM. But it's not that predictable in solidity, right? Because for example, the division by zero behavior already changed in solidity while it didn't change in EVM, right? And so we can't guarantee that overflow behavior will not change in solidity in the future. Despite that in EVM it will stay the same. But here we rely on the particular overflow behavior instead of preventing overflow at all. And in terms of gas cost, it will be the same. I mean preventing overflow or detecting it afterwards. So I wanted to point out something that transfer from isn't doing, which is what I consider at least the anti-pattern of requiring, sorry, not transfer from approve of requiring someone to set their approvals to zero before they can set them to another value. That was mooted in result of the issue with sort of front running with transaction approvals. And in my opinion the correct way to handle that is for end user clients to send an approval to set it to zero first and then just to reset it, not for contracts to enforce it. Because... Can I just give some context on that? So it's basically if Nick approves that I can use 10 of his tokens. And then at some later point he says no, Martin, he should only be able to spend five of them. And I see his approval is on the network for five tokens. So then I quickly grab the 10 tokens and his approval comes in. And I can grab another five tokens, then I got 15. And the correct way to handle that is either with first setting the app, either first setting the approval to zero and then back, but not for the token contract to enforce that. Because there are for instance smart contracts that use the approvals mechanism. They're doing that atomically in a single transaction, so they're not vulnerable to this race condition. But they will nevertheless be broken by a token that does this and possibly irrevocably broken. Ja, the contract has increased approval on the approval, but then it also has a peru. So it would have been great to just no-load a peru. Then it wouldn't be C20 of course. So this increased approval, is that standardised in a way? Not really, it's a de facto standard. That was an audience question too. For example, if I have an issue, I see a transaction that reduces my approval to zero. So wouldn't I just ask whether those can go things anyway? Yes, and nothing can stop you from doing that. It prevents, if it was ten and I'm setting it to five, then without this mitigation you could spend a total of 15. Because you could spend the ten and then I re-approved you for five, and then you spend the five. It also prevents you from the other direction. Even if you were approved for ten, I changed it to approve you for 20. In this scheme you could take a 30, right? Because you take that initial ten and then when I bump it up to 20, you're actually able to take it. So it works in both directions. Right, so setting it to zero and raising it again to five has to be two separate transactions. I think in response to the tweet was the official submission venue. Yeah, we can probably post it somewhere afterwards as well. But I think he wants to know how he can draw a submission to our attention now. Oh, is there a submission somewhere in some queue? Yeah. Cool. We'll come for the U.S. security. Right. We'll go through this a bit more. Until, because I think there were some, and then we'll jump to the next one. This is the actual contract implementation, and not just boilerplate. Well, there is actually mostly boilerplate. In the constructor it says issue cards for, what is that, from line 255. There's a hundred of them. There's a hundred of them. And every call to issue card here in this constructor will increase the total supply to 100 and set the balances of these hardcoded addresses. And then there's this, and that's an internal thing. So no one can call that externally. And then we have the buy card. There's actually one quick thing I want to point out about the issue card. On this part they actually did write, oftentimes in the constructor people allocate tokens and don't call transfer, which means that there's no transfer events. So if you are writing a tool that works against status or one of those other apps that looks at the history to figure out how many tokens you have, if you were allocated five tokens, for example in the constructor, and a transfer never happened, and you're just looking for those events, you don't actually know that you had five tokens. So that was... Right, from zero to whoever... Yeah, and it's also good like a reminder that the transfer event is like merely a helper. So a lot of apps just kind of assume the transfer event is the absolute truth, but it's good to always at least check on-chain whether that transfer event represents reality. And one other quick comment I might make as a shameless plug is there's a system called Merkle Air Drops that might have worked better here. For a hundred maybe not so bad, but you can imagine you don't want to, if you're putting this out into a thousand people's hands, not everyone maybe use their card, it makes more sense to Merkle-ize it and just submit the Merkle hash, and then force people to claim it. If that works in your case it might not work in all cases, but I could imagine situations where deploying this, especially if you want to issue 10 million cards would be infusible. Or a couple more audience questions. Sorry, I use the microphone because I still have it. I think that issue card has a bug here, because it can lead to inconsistent total supply, because if you issue card to the same address twice, then the balance will be still one, but total supply will be increased twice. Yes. Since others are coded, this can be discovered at this stage where we're at. Total right, but in itself that is flawed. You also bring up another good point. If I was doing this, I would probably sort the card, so by inspection you could tell easier if there was a duplicate one. Yes, I think this is a good, I think regardless I would change this, because this is an example of very fragile code. If the guide decides, oh actually, I don't have a hard coded list, I wanted to be dynamic, and then he changes this, and then he forgets to say, oh issue card wasn't made for that, and he'll deploy that, and then somebody buys something once, and then he'll lose one of them. It doesn't really cost, it would probably end up being like five more op codes, but it's good. Yes, it would be like a balance of two dots, and then plus. Always try to make your code as antifragile as possible, just because it works. If it's wonky, but it works in this specific situation, just change it so that it works in the general case, because you'll forget to change it later on. Having been a security auditor in general IT security, one thing that I've learned is that ask a lot of stupid questions, and don't be afraid to look stupid, because it makes things go faster if you just spit out things you don't understand. And I actually don't understand really how buy card works, because here money comes in, checks some stuff, and it emits an event, someone bought a card, and then sends the money out to somewhere else. It doesn't actually make any stories, changes about who owns the cards. So there's no like... I have a little external context on this. The author of this contract is present at DEVCON and is selling physical cards. If you transfer one ether using the buy card function with a hash, and then you provide him in person with the preimage, he will give you the physical card. Right, so it's a one-off purchase physically that you get the card in. So that's where there's no actual state change, meaningful state change in the buy card function. Sorry. Along the question of asking questions, it may be silly, but use of a preimage for securing an asset means that you may be front-run because it's a symmetric secret, meaning that if you observe someone attempting to claim this and you're not modifying the state of the actual contract, someone else can observe the fact that you've omitted that transaction and attempt to perform the same action to claim the card. So that would be a concern. In this specific case, all you would be doing is just paying for someone else's card. Because I think the data isn't exactly a secret, like it's not meant to be a secret, right? Well, it's a hash of a secret. Right, so I mean, if you submit somebody else's hash of a secret, then you just wouldn't be able to reveal it in person. Yes, and if you're talking about front-running the reveal, then in this case this is an in-person reveal, so you would have to literally jump in front of them and shout the secret. I misunderstood the intent then, but yes. But yeah, it's a good point, like about commitment reveal schemes in general, you need either the commit needs to contain some data that makes it impossible for someone else to front-run it, such as the target address, or you need to use something other than just a simple hash reveal, like using a key pair. So quick question. I didn't realize the context of this contract, but you just mentioned that how does the owner actually issue the card to the person? He hands it to them in person. I mean, but in the contract, or they don't. They're buying a physical object. Each of these cards he's handing out has a key pair on it, and each of those cards is listed in the contract here and owns one token. And then he will physically give you the physical card, and you will now own the key pair that owns the token. What is this token actually used for? Representing that you have a card. Yeah, so that seemed misused to me. Okay, but that means all cards are fungible? Yeah, but they should probably be NFTs. Yeah, that's a weird card collection system. All cards are the same, basically. I'd just like to mention one more thing on this contract before we can move on to the next. And if one of you guys can start bringing up the next contract, so we can switch over to that. This I think is very good to have an emergency drain on a contract, because you never know what people are going to send to it. So even if you don't expect, I mean you can prevent getting ether, but you can't prevent getting tokens. So if the tokens wind up there, it's nice if the owner can ship them off somewhere else. And I think this kind of also nicely demonstrates a minor pet peeve that I have with Solidity. And it is that owner, in this case, is an address. And doing the owner.transfer actually means doing a call with the next to no gas, which is not vulnerable to reentrancy. And the transfer also protects it. If it fails, it will throw. Whereas in this style, where it's an ERC-20 interface, it looks oddly similar. I mean it's something.transfer, but actually this is a full-blown call with all available gas. And totally different semantics. It also has a return value, which you might or might not read out. And as an auditor, when you're doing a manual audit, you have to be really careful about all these things. Is this an address, or is this an address in the geese of an interface, and actually a full-blown call? That's kind of a thing that you can trip on. Or wasn't there like a talk of deprecating that? Or no? I'm not familiar with that. Okay. So transfer was a recent introduction to the address object. How will you transfer to an address in the future? Oh, I see. Okay, so for anyone that didn't hear, there will be a new data type payable address, which you can cast an address to, and it only has that method if it's a payable type. Sorry? So the comment is you have to make it explicit in the constructor for that to happen. But what do you mean in the constructor of what? If you have an address as an object. If you have, if you call that method within like a function, and the function itself does not have payable, it won't be registered. Like it won't allow that to actually happen. No, you can always send funds out from a non-payable function. We're talking here about whether you can send, calling the transfer method on an address, and apparently in the future there will be an address subtype, and only that will have the transfer function to... Right. Right, so this is, I know with the right terminology, maybe it's a keyword, but it's like oddly... So in this case it's a key, it's a synthetic thing which has been added, syntactic sugar on the address type. And this type is a method, with an actual concrete method called transfer, on an external account. And for security there are very different semantics, and they matter a lot. Yeah. So I've had a couple of people send me things in Tweet DMs. I only have my phone here, so please send them to other people as well. Yeah, while I set this up, you just describe what the contract does, if you could. Yeah, so hi everybody, my name is Brennan. We've created a work contract. We've created a work contract so that you can deposit tokens in it, and then be eligible to be selected to complete work. So you say deposit a thousand tokens, and then now you can start working in some capacity. So it's still a work in progress, and I think it's going to get shredded. But the idea is that you deposit a certain amount of tokens, and then the work manager is able to withdraw from that deposit for jobs. And as soon as that deposit drops below the size of the jobs requirement, like the stake, then you'll become suspended. And so that forces you to deposit back into the contract to refill your work tokens. So if you can imagine, it's like the work deposit, that is the number of jobs you can have on the fly, if you will. So there's a few things that we haven't implemented, like being able to withdraw, or sorry, having a restriction on the timer, but I'm not sure if it's absolutely necessary, so I guess I'll hand it off to you guys to have a look. So is there a new token? I've actually left that out. Just ERC 20, so you can just instantiate it. So will this thing be dealing with assets of value, such as Ether or whatever valuable token? Just a custom work token. So at this point, are there any assets that have value? No, just the inherent value of being part of the work contract and participate in the jobs. So the value exchange is outside of that, and you become eligible to take part in that value network by staking tokens in the work contract. Who are the different actors in this model? So you have someone who does work. Yes, so you've got the worker who stakes the initial deposit. You have the job manager who's responsible for pulling out the job deposit, if you will, which is going to be smaller than the initial deposit, and then of course you have the owner of the contract you can deploy. And the owner of the contract is this, can he do administrative stuff on the contract? Like what? If I recall, this was a couple of weeks ago, I believe it's just instantiating it, and then there might be... They can set the stake limit, for example. Right, yeah, there's a few public variables that you can adjust. So hopefully it's small enough for you guys to grok. Okay, so let's see. Where would a user... What would be the first action a user does? So you deposit a stake. Okay, so let's go... It's quite hard a lot. I have to look at three different things. So the owner is just like an admin that manages this. Okay, so... Okay, so the owner deposits their stake, and it seems like they get tokens, the deposit, I mean sent back to them, right? Yeah, and just to add to this, so the worker has to approve the transfer before this deposit stake is called. So one thing I notice in the deposit stake is it doesn't seem to care whether it actually succeeds, like token.transfer from... You might have the allowance, but you can imagine there could be other restrictions like can't send to a contract address or something weird. Or you might just not have enough tokens. Right, absolutely. Here's one of these cases where we have this ambiguity, right? Where if this token is actually an implementation of the ERC-20 specification, which conforms exactly to the letter of specification, and this transfer from fails and returns a false, we just don't care about that. Yeah, so it's a case of being liberal with what you accept and careful with what you omit. If you call transfer, you should always wrap that in a require. Great, yeah. Yeah, the check for the allowance isn't necessary because the transfer call will fail if you don't have an allowance. And in this case, the check for the allowance is actually called out to another contract. So there's a minimum of 700 gas plus state reading fees. Right, yep. So yeah, always wrap your important stuff in requires. Which again is the sort of thing you're talking about because transfer and solidity explicitly doesn't require you wrap it in require. Yeah, so it's okay. Also one thing here, so since that deposit stake calls the deposit function, if you're going to import safe math and use it in some places, best to use it everywhere consistently, at least in my opinion, because otherwise it's, if you get used to seeing mixed code, mix some safe math and sometimes otherwise not, but also to become too easy for you to look at a line that needs safe math and then just ignore it because you're just used to, like the cognitive overload of keeping track of what, which threshold am I using safe math on, it can easily get mixed up. As a more generic thing, I want to point out as well, like a lot of auditing is just kind of getting your head wrapped around what the contract is trying to do in the first place. Going back to the description. Ja, that's kind of why being an effective auditor ties well together with not being afraid to ask stupid questions. Right, exactly. Yeah, yeah, oftentimes like the biggest vulnerabilities are the ones that are most obvious or the ones where like you're reading the code and it kind of, it's structured in a way to make you believe something that isn't actually true. And so when you get that inclination to be like, oh should this really be this way, it's important to voice those concerns and actually act on them or suppose being, oh I'm sure that's fine, nobody would get that wrong. We've seen some pretty drastic outcomes based on that kind of mindset. Two lines being flipped. Yeah. So the error messages are like the other way around, so they should say stake is over the limit, not stake is below the limit, because that's what it throws when it fails. So my partner agrees with you, I like to express what the boolean is, but that's I feel like a philosophy. No, no, no, this is an error message that will be thrown, right? Just tell what went wrong. But he's stating what condition was violated. I agree with you, but I can see the other side at least. Because you can imagine the output would say fail and then explain what was supposed to happen. This is a question going back maybe like one or two steps when we're talking about wrapping the transfer function in a require. Is that like a general good practice for when you're making a call out to another contract function at the require if you don't have some other mechanism to check to be sure that it succeeded? Yeah, so like, yeah, I mean, that's a good question. So like, definitely don't take from this that any time you're always calling out, you should always wrap it in a require because that in itself can create vulnerabilities, right? Like if there's a contract, for example, that iterates over a bunch of addresses and then wants to give a payout to each of those addresses, right? If one of those addresses is a smart contract that's malicious, it can just throw the moment you give something it and then you send something to it and then all the other payouts are stuck because of my malicious contract and I could blackmail you with that saying, now you're all going to send me one ether, otherwise I'm going to block your two ether payout forever, right? So you should always, first of all, try not to do that loop construct, which is bad, but besides that, you should wrap things around requires if and only if that line failing is just critical damage to your contract, right? If this line fails, then nothing should happen, right? Otherwise try to catch the return, right? If there is a return. You need to be very context aware. Yeah, it looks like Martin has something to do. Yeah, so I think, isn't there an overflow here in deposit, which is an internal function but an overflow, because the amount is U in 256, which arrives from a couple of lines above, and it is addition there without overflow checking, it will overflow, the new balance will be below the stake limit, or well, not necessarily. Or maybe necessarily. From a quick look through, I think all the place is deposited called the amount is set by the smart contract so it can ensure that plus total supply is not overflow, but it speaks to talking about making anti-fragile code where you should add these checks even if you don't think they can happen. It also makes the code more readable, even if it's going to be guaranteed it's nice from a readability point of view. But just scroll up a bit so we see where it's called, the deposits. So there's one deposit jobs to take as well. Yeah, it's called a few times. So that calls deposit, but required stake there comes from a state variable contract. And I guess that's the only one. There as well. It was required. Several calls to it, but in each case I think it comes from a state variable. So given the context it's fine, but it's not robust. Yeah, and I think this is also another good point. I always sort of suggest that if you have multiple functions, like calling a lower level function or like an internal function, and there's some security issue that could arise if those wrapping functions do not do a performance or an action, in this case making sure that the allowance is within a certain value, then you should absolutely move the check or the constraint to the shared function. So in here for example, by putting safe math here, you take away an overflow of vulnerability from any function that uses this function. And so when you're extending in the future, when people are building against your API, they don't have to have the cognitive overhead of oh, this function is vulnerable to overflows. So if there's a mitigation that you can build in, where there's an issue that's suffer, that multiple functions may suffer, always put that mitigation in the most deep function call. Another question. There's a function there, select worker. I assume the goal is that that gets called off chain. The select worker function. Does that get called off chain? No, it does get called on chain by something else that supplies the entropy. By another smart contract then? Essentially the job manager. One thing to bear in mind there is you may not be selecting uniformly at random, because if the range of your random values isn't a modulo of the size, so probably a better approach would be to expose the number of stakers and expose the way to index the stakers and then let the caller figure out how to select uniformly at random, because this hides the fact that it's doing modulo and you think it magically gets an even distribution. If you google modulo bias, you'll find a nice concise way to take the backs minus the modulo minus the size. It's a little more complicated, but you defeat modulo bias. And of course, if you're doing the checks on chain, doing the pseudo random generation on chain, then all the usual caveats about random numbers on chain. Yeah, and this is also vulnerable to front running if you can, you know, increase the length of the stakers before that select worker gets a call. Yeah, so if you're off by one from being chosen, you could, yeah. So I have a question. Will you be using some KYC system for the workers? Okay, it depends, because if you want the stake limit, it does nothing, it's not like the stake limit, you know, to limit the maximum stake, so maximum chance of each worker can be get bypassed by just registering with multiple addresses inside the stake limit. So it's not simple resistant without the KYC system, right? It's funny, we actually explored that because we were considering letting people have a lot of stake and then wait the workers proportionally according to how much stake they have. But computationally, that's not, that's very challenging, because basically this is 01 for all the operations and at best it'd be ON depending on how you want to balance read optimization versus write optimization. So we decided that KYC be damned, I guess. We just let them, if they have a million, they can stake that many addresses. But again, I think it's kind of outside the scope of this. It depends on how you want to use it, but you bring up an excellent point, I think. I mean, the stake limit, the question was wouldn't it be better to get rid of the stake limit then so you can stake as much as you want? And that's still an unopened question that we've been talking about, because really, if you can just only do one job at a time, then really you only need the job stake amount minimum. Yeah, and I think it's something to keep in mind in general when you're doing these sorts of things is to keep in mind, because that's definitely an issue. Like when people can be, when many people could actually be one actor. And this is a pattern we do see a lot because people just trusting, no, no, one address means one person, but like you said, if you have a million addresses and there's only two other workers, you have a one million out of a million and two chance of being picked for this thing. All right, should we? So we got one tennis submission that we could take a look at that. We have like 10 minutes left. Alternatively, we could just talk about our favorite pet peeves or whatever. Thanks guys, by the way. Thanks guys. Yeah, thank you for coming on stage. Yeah. There you go. Yeah, so this is pretty small, pretty straightforward, and I wanted to kind of kick off the discussion. I mean, I think all of you will immediately see what I'm getting at. Line 21. So, I mean, given that there was some changes in the new open zeppelin version 2 and so on in this direction, I wanted to kind of kick off the discussion towards reentrancy protections and so on. And what all you guys think about that, given that you were also partially involved with that and so on. Yeah. So I think the first thing is that this contract is, you know, like just generally stop doing this, you know. Don't check if things are contracts. There is no way to do this on Ethereum that's foolproof. There is. No. No, no, no. Well, actually. You can prove that it's not a contract by checking that the message of sounder equals origin. Sure. But you cannot prove that it is a contract. Like when the big issues is kind of factual, I can compute the address a contract will live at, and now I can use that, but in the future it becomes a contract. So. Right. The tx origins equal. Yes. And. But I think in general it's, it's, I think it's an anti pattern because we want to build a system that is interoperable. And if you can't build a secure contract, unless it's being called by an externally undercount, then you need to rework your contract. So it is. Right. And. That's one of the mechanics of this. Nick, you want to. Yeah. So if you call self destruct, the contracts code size will be zero, even though the contract is still callable for the rest of the transaction. So you could exploit this by having your contract self destruct itself and then call this and then do re-entrancy and then die afterwards. No, hold on. No, no, no, no. No, the code size will not be zero. Am I getting it back to the end of the. Yeah. The code size will still be the actual code size until after the, but the constructor, that's it. Yeah. Because the construct during the constructor. Right. Then the code size of that address will be zero. And I would still not, I don't know, the TX origin thing doesn't make, it's not a comfortable sensor for me long term because like for example, what if we have to text abstraction in the future, right? Like the, you know, things coming from the onsite address that there may be actually be code there because it may count the, depending on how it's implemented in the future, it may count the stub us code. There is actually, I think maybe only one full proof, future proof way to do is, which is check the hash of message sender against a signature of message from that address, right? Because smart contracts, obviously anything that is a public contract can have a private key. So if you make sure that you get a signature from whoever is calling, then that's like a, I would assume a full proof way. Probably. Yeah. Who knows what crazy moon math will have in the future. So. At. It's really important. You can also just check the extra proof size on the second trend. I mean, I think in this particular case, it might not actually be exploitable because it then relies on calling the thing as asserted wasn't a contract. So if it's still in the constructor, it has no code and you can't call it back yet. But. Oh, right. Again, I'd still say. To do explain, I didn't catch that. So what was the idea? No, but again. Anyway, the idea is that the first transaction they call in and they set up a way or something like that. Yeah. In this case, if message sender has no code, it still works because it's just a rally transfer. Yes. But it can't be exploited for re-intrancy, which I assume is what the author was worried about. I do have one quick question, actually. Can you set the init code to be empty so you can actually have a contract with no code? Yes. You can just literally return zero bytes. Great. So that idea, by the way, that depends on the idea that for a later transaction, they cannot have zero code size again because the constructor will not be run again. And if it's in a different block, that is true for about another month. But then we have create2 coming, which kind of changes the semantics and the pattern of contract creation a bit. So that, and it will have quite an impact on security, on the security aspects of how you deal with external contracts and assumptions that you can make because it will be possible to take down a contract and replace it with something completely different on the same address through the use of loading the actual body code from an oracle on chain in the constructor. Because the address is determined from the init code and not from the actual runtime byte code. And actually, I take back what I said before. A smart contract, there is technically an extremely statistically unlikely possibility that you can create a smart contract address that you have private key to. There's collisions in the namespace. And it meets yours simultaneously as every Ethereum practitioner in the world unlikely. Yeah, yeah, but I mean, you know, possible. Yes. Can I just follow up on that? Can you explain a little bit better? Is that within the realm of reason, a reasonable way to check it? Yeah, I mean, it's like getting like a legend is like, you know, one in ten billion universes. Please don't do it. Please, please don't do it, though, because it's systems are much more useful if they can be interacted with by other systems than if you try and enforce that they can only be interacted with by users. So, yeah, that's what it is. For example, open zephyr now has the reentrancy guard. Shouldn't it be used? Or are we kind of giving up on security because we are using the reentrancy guard and limiting that we are incapable of reality. We are writing like reentrancy. Is the reentrancy guard structured like this, where it can only be called by external code? No, it's countered. So, I would call like a mutex, like a last resort. It's better than this, but it's still like you should be able to write your code without mutexes. If you really can't, then actually mutexes will get more efficient in the next hard fork because of net gas metering. Oh, yeah. Yeah, but be careful. Everyone read up on create2 because that's going to break a lot of assumptions, you guys. Yeah, I open up a lot of opportunities as well. So, there are like only a couple of minutes and the next team are back to back. So, I don't know, can I have like one more question or so? So, why wouldn't this contract be vulnerable to reentry? Hvis I just call deposit and send in the constructor of the contract? Yes, but if you call send, then it will try and call you back and because it's in the constructor, you don't get any code at that address, so it won't be able to be reentrant. Yeah, the code, the way that Ethereum works is that the constructor returns a string of EVM bytes and only at the end of the transaction are those connected to the contract. Thanks. For one more. What's the best way to, with transferring some value to set the gas limit? The easiest way is to use dot transfer, which sets it to the minimum, which is 2100. If we want the dynamic, if we want to choose the limit and not have the 21000. In Solidity you can do a function named dot gas in brackets and then second set of parentheses for the function arguments. But the thing is, even if you do, if you provide zero, there's always the 2300 gas. Only for value setting transactions. If you have a value. You have to send it at least one way. Yeah. But generally it's friendly to send at least 2300 gas even if you don't have to because that way they can log events, which means otherwise it can have no effect whatsoever. I think that's all we have time for. Yes. Thank you guys. Thank you audience.