What's the purpose of introducing trailing-return syntax here? What role does auto serve, apart from aligning the names? Seems aggressive...
uint32_t GetVersion() const { return version; }
const std::vector<CTxIn>& GetInputs() const LIFETIMEBOUND { return vin; }
const std::vector<CTxOut>& GetOutputs() const LIFETIMEBOUND { return vout; }
uint32_t GetLockTime() const { return nLockTime; }
More broadly, as @mzumsande also mentioned, I am not convinced that introducing new names for vin, vout, version, and nLockTime is an improvement. Could we keep the protocol names instead?
uint32_t version() const { return version; }
const std::vector<CTxIn>& vin() const LIFETIMEBOUND { return vin; }
const std::vector<CTxOut>& vout() const LIFETIMEBOUND { return vout; }
uint32_t nLockTime() const { return nLockTime; }
With indexed helpers, the diff becomes even closer (and safer) to the current code:
const CTxIn& vin(size_t index) const LIFETIMEBOUND { return m_vin.at(index); }
const CTxOut& vout(size_t index) const LIFETIMEBOUND { return m_vout.at(index); }
For example:
- if (tx.vin.empty())
+ if (tx.vin().empty())
- if (tx.vout.empty())
+ if (tx.vout().empty())
- for (const auto& txout : tx.vout)
+ for (const auto& txout : tx.vout())
- for (const auto& txin : tx.vin) {
+ for (const auto& txin : tx.vin()) {
- if (tx.vin[0].scriptSig.size() < 2 || tx.vin[0].scriptSig.size() > 100)
+ if (tx.vin(0).scriptSig.size() < 2 || tx.vin(0).scriptSig.size() > 100)
- for (const auto& txin : tx.vin)
+ for (const auto& txin : tx.vin())
That would make the mechanical diff much easier to review manually.
The renaming question can be debated separately, but as-is the change reads like a broad spelling rewrite with non-negligible review cost and little immediate benefit.
