mintlayer / core

Mintlayer Core
https://www.mintlayer.org/
MIT License
14 stars 4 forks source link

utxo: Initial support for unsigned transactions #121

Closed iljakuklic closed 2 years ago

iljakuklic commented 2 years ago

This uses unsigned transactions for UTXO transactions. Avoids the problem with fees coming off of the pallet_balances funds. The docs from Parity [1] suggest that some extra anti-spam measures are required to let the transactions through but that is not what I observed. Seems to work for me like it is in this patch. Not yet ready for merge, some cleanups and more testing required.

[1] https://docs.substrate.io/v3/concepts/extrinsics/#unsigned-transactions

b-yap commented 2 years ago

I ran 5 nodes:

RUST_LOG=info ./target/release/mintlayer-core --base-path /tmp/bob --testnet --name bob --port 30333 --ws-port 9945 --rpc-port 9933 --validator --rpc-methods Unsafe

three of these:

RUST_LOG=info ./target/release/mintlayer-core --base-path /tmp/charlie --testnet --name charlie --port 30334 --ws-port 9946 --rpc-port 9934 --validator --bootnodes /ip4/127.0.0.1/tcp/30333/p2p/12D3KooWFYxf6U7wwWCc5Tc8e39NVQwsGy8SBCQs2hVch3qh24kx

and 1 extra:

RUST_LOG=info ./target/release/mintlayer-core --base-path /tmp/extra --testnet --name extra --port 30337 --ws-port 9949 --rpc-port 9937 --validator --bootnodes /ip4/127.0.0.1/tcp/30333/p2p/12D3KooWFYxf6U7wwWCc5Tc8e39NVQwsGy8SBCQs2hVch3qh24kx

And I have 2 extra accounts created from polkadot.js. Tested:

  1. Our Alice funding these 2 accounts connecting to port 9949
  2. extra accounts sending funds to each other through port 9949

Spend successful , with these 2 extra accounts having 0 pallet-balance

TheQuantumPhysicist commented 2 years ago

Apart from the many unused variables. This is a draft. So that's OK.

So there's nothing in substrate that's affected by this. We didn't have to modify spam protection. We didn't have to tell substrate, at any capacity, that fees shouldn't be taken from outputs instead of from extrinsics. This implicitly means that substrate (still) doesn't have a way to prioritize which transactions come first and which don't.

Does this mean that we're gonna have another PR that covers spam protection and tx prioritization in mempool based on fee?

Another thing I'd like to see in this PR is that transactions with origins should be rejected. This will have very interesting implications on staking, I imagine. Would that work?

TheQuantumPhysicist commented 2 years ago

Please rebase and ensure CI tests pass before merging. For some reason I see the merge button enabled just because people approved (with no warning, since usually I can force-merge as admin). Do you guys see that? If that's the case, I'll try to find a way to fix it. I expected it to be enabled only if the CI builds passed.

muursh commented 2 years ago

@TheQuantumPhysicist I can fix that

muursh commented 2 years ago

Done